Re: [HACKERS] Re: [BUGS] BUG #13611: test_postmaster_connection failed (Windows, listen_addresses = '0.0.0.0' or '::')

[Tatsuo Ishii]

Noah,

> No, PQping("host='127.0.0.1'") fails to reach a listen_addresses='::' server
> on many systems.  Here's what I thought Kondo was proposing:
> 
> --- a/src/bin/pg_ctl/pg_ctl.c
> +++ b/src/bin/pg_ctl/pg_ctl.c
> @@ -649,5 +649,9 @@ test_postmaster_connection(pgpid_t pm_pid, bool 
> do_checkpoint)
>  
> - /* If postmaster is listening 
> on "*", use localhost */
> + /* explanation here */
>   if (strcmp(host_str, "*") == 0)
>   strcpy(host_str, 
> "localhost");
> + else if (strcmp(host_str, 
> "0.0.0.0") == 0)
> + strcpy(host_str, 
> "127.0.0.1");
> + else if (strcmp(host_str, "::") 
> == 0)
> + strcpy(host_str, "::1");
>  

I see. Would you like to commit this?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


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

Re: [HACKERS] Re: [BUGS] BUG #13611: test_postmaster_connection failed (Windows, listen_addresses = '0.0.0.0' or '::')

[Tatsuo Ishii]

Noah,

> No, PQping("host='127.0.0.1'") fails to reach a listen_addresses='::' server
> on many systems.  Here's what I thought Kondo was proposing:
> 
> --- a/src/bin/pg_ctl/pg_ctl.c
> +++ b/src/bin/pg_ctl/pg_ctl.c
> @@ -649,5 +649,9 @@ test_postmaster_connection(pgpid_t pm_pid, bool 
> do_checkpoint)
>  
> - /* If postmaster is listening 
> on "*", use localhost */
> + /* explanation here */
>   if (strcmp(host_str, "*") == 0)
>   strcpy(host_str, 
> "localhost");
> + else if (strcmp(host_str, 
> "0.0.0.0") == 0)
> + strcpy(host_str, 
> "127.0.0.1");
> + else if (strcmp(host_str, "::") 
> == 0)
> + strcpy(host_str, "::1");
>  

I see. Would you like to commit this?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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: Implement failover on libpq connect level.

[Shulgin, Oleksandr]

On Mon, Oct 26, 2015 at 10:02 PM, Christopher Browne 
wrote:

>
> On 26 October 2015 at 16:25, Peter Eisentraut  wrote:
>
>> On 10/14/15 6:41 AM, Victor Wagner wrote:
>> > 1. It is allowed to specify several hosts in the connect string, either
>> > in URL-style (separated by comma) or in param=value form (several host
>> > parameters).
>>
>> I'm not fond of having URLs that are not valid URLs according to the
>> applicable standards.  Because then they can't be parsed or composed by
>> standard libraries.
>>
>> Also, this assumes that all the components other than host and port are
>> the same.  Earlier there was a discussion about why the ports would ever
>> need to be different.  Well, why can't the database names be different?
>>  I could have use for that.
>>
>> I think you should just accept multiple URLs.
>>
>
> I'd give a "+1" on this...
>
> As an area of new behaviour, I don't see a big problem with declining to
> support every wee bit of libpq configuration, and instead requiring the
> use of URLs.
>
> Trying to put "multiplicities" into each parameter (and then considering
> it at the pg_service level, too) is WAY more complicated, and for a
> feature where it seems to me that it is pretty reasonable to have a
> series of fully qualified URLs.
>
> Specifying several URLs should be easier to understand, easier to
> test, easier to code, and easier to keep from blowing up badly.
>

Setting aside all other concerns, have a +1 from me on that too.

--
Alex

Re: [HACKERS] TODO list updates

[Heikki Linnakangas]

On 16 October 2015 18:20:59 EEST, Bruce Momjian  wrote:
>I think on-disk bitmap indexes would only beat GIN indexes in a
>read-only database on low-cardinality columns.  For example, if you had
>a purchase_log table and wanted to know all the "blue" and "large"
>items
>sold at a specific store, I can see on-disk bitmap indexes doing well
>there.  If you added the product number, or needed read/write, I think
>GIN would win.  I just don't think we have enough deployments who need
>what on-disk bitmap are best at.

My take on this is that we effectively already have bitmap indexes: it's called 
GIN. We could make the posting list compression even better, currently a TID is 
compressed at best to a single byte, while in a bitmap index it could go down 
to one bit, or even less. But that's just a matter of improving the compression 
algorithm, making it more bitmapy, and could be done as a fairly isolated 
change in GIN code.

Besides being more dense, there are some other tricks often associated with 
bitmap indexes. Instead of storing a bitmap/posting list per each unique value, 
you could store one for a range of values. That's useful e.g. for storing 
floats, where you don't have many exact duplicate values, but you could get a 
dense index by treating all values in range 0.0-10.0 as one entry, all values 
in 10.0-50.0 as another,  and so forth. Yet another trick is to have one bitmap 
for all values > 0.0, another for all values >10.0, and so forth. With that, 
you can satisfy any BETWEEN query by scanning just two bitmaps/posting lists: 
the one for the lower bound and the one for the upper bound. The matching 
tuples are the ones that are present in the first, but not the latter posting 
list. But that's also not a whole new index type. GIN could do all that, if 
someone just wrote an opclass for it.

- Heikki


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

Re: [HACKERS] Re: [BUGS] BUG #13611: test_postmaster_connection failed (Windows, listen_addresses = '0.0.0.0' or '::')

[Noah Misch]

On Mon, Oct 26, 2015 at 09:54:03AM +0900, Tatsuo Ishii wrote:
> > Kondo's design is correct.
> 
> So more proper fix looks like this?

> + /* If postmaster is listening 
> on "*", "0.0.0.0" or "::", use 127.0.0.1 */
> + if (strcmp(host_str, "*") == 0 
> ||
> + strcmp(host_str, 
> "0.0.0.0") == 0 ||
> + strcmp(host_str, "::") 
> == 0)
> + strcpy(host_str, 
> "127.0.0.1");

No, PQping("host='127.0.0.1'") fails to reach a listen_addresses='::' server
on many systems.  Here's what I thought Kondo was proposing:

--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -649,5 +649,9 @@ test_postmaster_connection(pgpid_t pm_pid, bool 
do_checkpoint)
 
-   /* If postmaster is listening 
on "*", use localhost */
+   /* explanation here */
if (strcmp(host_str, "*") == 0)
strcpy(host_str, 
"localhost");
+   else if (strcmp(host_str, 
"0.0.0.0") == 0)
+   strcpy(host_str, 
"127.0.0.1");
+   else if (strcmp(host_str, "::") 
== 0)
+   strcpy(host_str, "::1");
 


-- 
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] Getting sorted data from foreign server

[Ashutosh Bapat]

On Fri, Oct 23, 2015 at 2:43 AM, Robert Haas  wrote:

> On Wed, Oct 21, 2015 at 5:23 AM, Ashutosh Bapat
>  wrote:
> > Increasing the sorting cost factor (when use_remote_estimates = false)
> from
> > 1.1 to 1.2 makes the difference disappear.
> >
> > Since the startup costs for postgres_fdw are large portion of total cost,
> > extra 10% of rest of the cost is comparable to 1% fuzzy limit. IMO, we
> > shouldn't bother too much about it as the path costs are not much
> different.
>
> My feeling is that cranking the sorting cost factor up to 20-25% would
> be a good idea, just so we have less unnecessary plan churn.  I dunno
> if sorting always costs that much, but if a 10% cost overhead is
> really 1% because it only applies to a fraction of the cost, I don't
> think that's good.  The whole point was to pick something large enough
> that we wouldn't take the sorted path unless we will benefit from the
> sort, and clearly that's not what happened here.
>
>
PFA patch with the default multiplication factor for sort bumped up to 1.2.


> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 697de60..cb5c3ae 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -186,23 +186,26 @@ is_foreign_expr(PlannerInfo *root,
 	 * Check that the expression consists of nodes that are safe to execute
 	 * remotely.
 	 */
 	glob_cxt.root = root;
 	glob_cxt.foreignrel = baserel;
 	loc_cxt.collation = InvalidOid;
 	loc_cxt.state = FDW_COLLATE_NONE;
 	if (!foreign_expr_walker((Node *) expr, _cxt, _cxt))
 		return false;
 
-	/* Expressions examined here should be boolean, ie noncollatable */
-	Assert(loc_cxt.collation == InvalidOid);
-	Assert(loc_cxt.state == FDW_COLLATE_NONE);
+	/*
+	 * The collation of the expression should be none or originate from a
+	 * foreign var.
+	 */
+	Assert(loc_cxt.state == FDW_COLLATE_NONE ||
+			loc_cxt.state == FDW_COLLATE_SAFE);
 
 	/*
 	 * An expression which includes any mutable functions can't be sent over
 	 * because its result is not stable.  For example, sending now() remote
 	 * side could cause confusion from clock offsets.  Future versions might
 	 * be able to make this choice with more granularity.  (We check this last
 	 * because it requires a lot of expensive catalog lookups.)
 	 */
 	if (contain_mutable_functions((Node *) expr))
 		return false;
@@ -1870,10 +1873,57 @@ printRemoteParam(int paramindex, Oid paramtype, int32 paramtypmod,
  */
 static void
 printRemotePlaceholder(Oid paramtype, int32 paramtypmod,
 	   deparse_expr_cxt *context)
 {
 	StringInfo	buf = context->buf;
 	char	   *ptypename = format_type_with_typemod(paramtype, paramtypmod);
 
 	appendStringInfo(buf, "((SELECT null::%s)::%s)", ptypename, ptypename);
 }
+
+/*
+ * Deparse ORDER BY clause according to the given pathkeys for given base
+ * relation. From given pathkeys expressions belonging entirely to the given
+ * base relation are obtained and deparsed.
+ */
+void
+appendOrderByClause(StringInfo buf, PlannerInfo *root, RelOptInfo *baserel,
+	List *pathkeys)
+{
+	ListCell			*lcell;
+	deparse_expr_cxt	context;
+	int	nestlevel;
+	char*delim = " ";
+
+	/* Set up context struct for recursion */
+	context.root = root;
+	context.foreignrel = baserel;
+	context.buf = buf;
+	context.params_list = NULL;
+
+	/* Make sure any constants in the exprs are printed portably */
+	nestlevel = set_transmission_modes();
+
+	appendStringInfo(buf, " ORDER BY");
+	foreach(lcell, pathkeys)
+	{
+		PathKey*pathkey = lfirst(lcell);
+		Expr*em_expr;
+
+		em_expr = find_em_expr_for_rel(pathkey->pk_eclass, baserel);
+		Assert(em_expr != NULL);
+
+		appendStringInfoString(buf, delim);
+		deparseExpr(em_expr, );
+		if (pathkey->pk_strategy == BTLessStrategyNumber)
+			appendStringInfoString(buf, " ASC");
+		else
+			appendStringInfoString(buf, " DESC");
+
+		if (pathkey->pk_nulls_first)
+			appendStringInfoString(buf, " NULLS FIRST");
+
+		delim = ", ";
+	}
+	reset_transmission_modes(nestlevel);
+}
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 65ea6e8..58bf76c 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -127,86 +127,82 @@ ALTER FOREIGN TABLE ft2 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
 (2 rows)
 
 -- Now we should be able to run ANALYZE.
 -- To exercise multiple code paths, we use local stats on ft1
 -- and remote-estimate mode on ft2.
 ANALYZE ft1;
 ALTER FOREIGN TABLE ft2 OPTIONS (use_remote_estimate 'true');
 -- ===
 -- simple queries
 -- ===
--- 

Re: [HACKERS] Proposal: Trigonometric functions in degrees

[Dean Rasheed]

On 26 October 2015 at 22:51, Tom Lane  wrote:
> Dean Rasheed  writes:
>> Personally I'd rather have sind(30) be exactly 0.5, even if
>> sind(29.999...) or sind(30.000...1) ended up the wrong side of it.
>
> TBH, sir, if you think that you are too dangerous to be allowed anywhere
> near any numerical analysis code.  Preserving mathematical properties like
> monotonicity is *far* more important than whether sin(30 degrees) comes
> out exact or not.  You can do proofs about the behavior of algorithms
> using these functions if you can ensure monotonicity; but exactness of
> values is always going to depend on subtle details of the floating point
> format.
>
> I think using a range reduction to some fractional part of the circle is a
> reasonable way of trying to deal with these concerns, but I really really
> do not want to see it special-casing point values in a way that doesn't
> guarantee monotonicity.
>

OK, agreed. Preserving the monotonicity of sind over the range 0..90
is important. It's useful having this discussion before implementing a
lot of code that people might come to rely on.


> It may be that guaranteeing the sin(30) case is just not very feasible,
> and we should content ourselves with getting the 0/90/180/270/360 cases
> exactly, which we could do with a mod 90 initial reduction.  Doing mod 30
> or mod 45 would require sewing together pieces of the curve that might not
> meet exactly, if we were unlucky.  (I've not tried it though.)
>

I think it's still feasible to have sind(30) = 0.5 exactly and keep
monotonicity. If we can implement sind over the range 0..90, the rest
is easy, and in that range there are 3 known fixed points that we want
to preserve -- (0,0), (30,0.5) and (90,1). So all we need is a way to
compute each half of the range, interpolating between the fixed
points. That ought to be possible by pre-computing the value of
sin(pi/6) and using it to scale the result of sin(x) to guarantee that
the pieces join in the middle.  (I've not tried it either, but it
sounds plausible.)


> What have you got in mind for tan() and the rest?
>

If we have sind() for the range 0..90, cosd() could be implemented as
sind(90 - x) after reducing x to the range 0..90, which would then
give it the right set of properties and it would be exact for 0, 60
and 90.

Then tand() and cotd() could be implemented as their ratios, which
would guarantee that tand(45) = cotd(45) = 1 exactly and that they are
monotonic nearby, without needing any special case code.

I haven't thought about the inverse functions yet, but they ought to
be possible in a similar way, albeit a bit fiddly.

Regards,
Dean


-- 
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: PL/Pythonu - function ereport

[Pavel Stehule]

Hi

2015-10-23 7:34 GMT+02:00 Catalin Iacob :

> On Sat, Oct 17, 2015 at 8:18 PM, Pavel Stehule 
> wrote:
> > here is new patch
> >
> > cleaned, all unwanted artefacts removed. I am not sure if used way for
> > method registration is 100% valid, but I didn't find any related
> > documentation.
>
> Pavel, some notes about the patch, not a full review (yet?).
>
> In PLy_add_exceptions PyDict_New is not checked for failure.
>
> In PLy_spi_error__init__, kw will be NULL if SPIError is called with
> no arguments. With the current code NULL will get passed to
> PyDict_Size which will generate something like SystemError Bad
> internal function call. This also means a test using just SPIError()
> is needed.
> It seems args should never be NULL by the way, if there are no
> arguments it's an empty tuple so the other side of the check is ok.
>
> The current code doesn't build on Python3 because the 3rd argument of
> PyMethod_New, the troubled one you need set to NULL has been removed.
> This has to do with the distinction between bound and unbound methods
> which is gone in Python3.
>

this part of is well isolated, and we can do switch for Python2 and Python3
without significant problems.


>
> There is http://bugs.python.org/issue1587 which discusses how to
> replace the "third argument" functionality for PyMethod_New in
> Python3. One of the messages there links to
> http://bugs.python.org/issue1505 and
> https://hg.python.org/cpython/rev/429cadbc5b10/ which has an example
> very similar to what you're trying to do, rewritten to work in
> Python3. But this is still confusing: note that the replaced code
> *didn't really use PyMethod_New at all* as the removed line meth =
> PyMethod_New(func, NULL, ComError); was commented out and the replaced
> code used to simply assign the function to the class dictionary
> without even creating a method.
> Still, the above link shows a (more verbose but maybe better)
> alternative: don't use PyErr_NewException and instead define an
> SPIError type with each slot spelled out explicitly. This will remove
> the "is it safe to set the third argument to NULL" question.
>
> I tried to answer the "is it safe to use NULL for the third argument
> of PyMethod_New in Python2?" question and don't have a definite answer
> yet. If you look at the CPython code it's used to set im_class
> (Objects/classobject.c) of PyMethodObject which is accessible from
> Python.
> But this code:
> init = plpy.SPIError.__init__
> plpy.notice("repr {} str {} im_class {}".format(repr(init), str(init),
> init.im_class))
> produces:
> NOTICE:  repr  str  SPIError.__init__> im_class 
> so the SPIError class name is set in im_class from somewhere. But this
> is all moot if you use the explicit SPIError type definition.
>

Should be there some problems, if we lost dependency on basic exception
class? Some compatibility issues? Or is possible to create full type with
inheritance support?


>
> >> Any help is welcome
>
> I can work with you on this. I don't have a lot of experience with the
> C API but not zero either.
>

It is very helpful for me - C API doesn't look difficult, but when I have
to do some really low level work I am lost :(

Regards

Pavel

[HACKERS] Disabling START TRANSACTION for a SuperUser

[rajan]

Hi,

I have created a readonly user by executing the following statements,
CREATE USER backupadm SUPERUSER  password 'mypass';
ALTER USER backupadm set default_transaction_read_only = on;

But the backupadm user is able to create/update table when using START
TRANSACTION READ WRITE and then COMMIT;

Is there any way to block/disabling an User from running Transactions?

Thanks in advance.



--
View this message in context: 
http://postgresql.nabble.com/Disabling-START-TRANSACTION-for-a-SuperUser-tp5871630.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Disabling START TRANSACTION for a SuperUser

[rajan]

Hey Craig,

Thanks for your response. Seems like the workaround is difficult.

I am trying to understand 
"
ExecutorStart_hook and ProcessUtility_hook, implemented with 
a C extension. You can find an example of one in pg_stat_statements, 
sepgsql, and in the BDR source code. The latter uses it for a similar 
purpose to what you describe - to limit what commands can be run. 
"

Let me see what i can do...

Thanks again...



--
View this message in context: 
http://postgresql.nabble.com/Disabling-START-TRANSACTION-for-a-SuperUser-tp5871630p5871645.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Getting sorted data from foreign server

[Fabrízio de Royes Mello]

On Tue, Oct 27, 2015 at 5:26 AM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:
>
>
>
> On Fri, Oct 23, 2015 at 2:43 AM, Robert Haas 
wrote:
>>
>> On Wed, Oct 21, 2015 at 5:23 AM, Ashutosh Bapat
>>  wrote:
>> > Increasing the sorting cost factor (when use_remote_estimates = false)
from
>> > 1.1 to 1.2 makes the difference disappear.
>> >
>> > Since the startup costs for postgres_fdw are large portion of total
cost,
>> > extra 10% of rest of the cost is comparable to 1% fuzzy limit. IMO, we
>> > shouldn't bother too much about it as the path costs are not much
different.
>>
>> My feeling is that cranking the sorting cost factor up to 20-25% would
>> be a good idea, just so we have less unnecessary plan churn.  I dunno
>> if sorting always costs that much, but if a 10% cost overhead is
>> really 1% because it only applies to a fraction of the cost, I don't
>> think that's good.  The whole point was to pick something large enough
>> that we wouldn't take the sorted path unless we will benefit from the
>> sort, and clearly that's not what happened here.
>>
>
> PFA patch with the default multiplication factor for sort bumped up to
1.2.
>

+/* If no remote estimates, assume a sort costs 10% extra */
+#define DEFAULT_FDW_SORT_MULTIPLIER 1.2

The above comment should not be 20%?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: [HACKERS] Disabling START TRANSACTION for a SuperUser

[Craig Ringer]

On 27 October 2015 at 18:25, rajan  wrote:
> Hi,
>
> I have created a readonly user by executing the following statements,
> CREATE USER backupadm SUPERUSER  password 'mypass';

A superuser can never be a read only user.

> ALTER USER backupadm set default_transaction_read_only = on;

They can just

SET default_transaction_read_only = off;

to get around that. It has no useful effect for security.

> But the backupadm user is able to create/update table when using START
> TRANSACTION READ WRITE and then COMMIT;
>
> Is there any way to block/disabling an User from running Transactions?

No, it's fundamentally impossible, because the statements you
mentioned - like CREATE USER - also run within transactions.

You could stop them from running an explicit transaction, but that
wouldn't stop them using CREATE TABLE, UPDATE, etc, as stand-alone
statements.

What you appear to want can be achieved, albeit with some difficulty,
using an ExecutorStart_hook and ProcessUtility_hook, implemented with
a C extension. You can find an example of one in pg_stat_statements,
sepgsql, and in the BDR source code. The latter uses it for a similar
purpose to what you describe - to limit what commands can be run.
Doing that securely will be challenging.


-- 
 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] Duplicated assignment of slot_name in walsender.c

[Alvaro Herrera]

Andres Freund wrote:

> If you want to do that, I'm unenthusiastically not objecting. But if so,
> let's also do it in IdentifySystem(), SendTimeLineHistory(),
> StartReplication(), SendBackupHeader(), SendXLogRecPtResult() - they're
> modeled after each other.

Okay, pushed with that, backpatched to 9.4 which is where it all applied
with no conflicts, to ease future backpatching pain.  (Some of this code
exists back in 9.3, but git generated a lot of conflicts in
cherry-picking so I didn't bother).

In SendXLogRecPtrResult() we now rely on snprintf's return value, rather
than doing a separate strlen call.  We do have some other places (not a
lot admittedly) in which we rely on snprintf returning correctly.  I
assume that's the norm when there's no possibility of failure.

I wonder why we use MAXFNAMELEN to print %X/%X -- that seems odd.

-- 
Á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] Rework the way multixact truncations work

[Andres Freund]

Hi,

On 2015-10-24 22:07:00 -0400, Noah Misch wrote:
> I'm several days into a review of this change (commits 4f627f8 and
> aa29c1c).

Cool!

> There's one part of the design I want to understand before commenting on
> specific code.  What did you anticipate to be the consequences of failing to
> remove SLRU segment files that MultiXactState->oldestMultiXactId implies we
> should have removed?  I ask because, on the one hand, I see code making
> substantial efforts to ensure that it truncates exactly as planned:

> [portion of TruncateMultiXact]

The reason we can't have checkpoints there, is that checkpoints records
multixact values in the checkpoint record. If we crash-restart before
the truncation has finished we can end up in the situation that
->oldestMultiXactId doesn't exist. Which will trigger a round of
emergency vacuum at the next startup, not something that should happen
due to a concurrency problem.

We could instead update the in-memory values first, but that could lead
to other problems.


So the critical section/delaying of checkpoints is more about having the
on-disk agreeing with the status data in the checkpoint/control file.


> On Tue, Sep 22, 2015 at 07:57:27PM +0200, Andres Freund wrote:
> > On 2015-09-22 13:38:58 -0400, Robert Haas wrote:
> > > - If SlruDeleteSegment fails in unlink(), shouldn't we at the very
> > > least log a message?  If that file is still there when we loop back
> > > around, it's going to cause a failure, I think.
> > 
> > The existing unlink() call doesn't, that's the only reason I didn't add
> > a message there. I'm fine with adding a (LOG or WARNING?) message.

Note that I didn't add the warning after all, as it'd be too noisy
during repeated replay, as all the files would already be gone. We could
only emit it when the error is not ENOFILE, if people prefer that.


> Unlinking old pg_clog files is strictly an optimization.  If you were to
> comment out every unlink() call in slru.c, the only ill effect on CLOG is the
> waste of disk space.  Is the same true of MultiXact?

Well, multixacts are a lot larger than the other SLRUs, I think that
makes some sort of difference.


Thanks,

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] fortnight interval support

[David Fetter]

On Tue, Oct 27, 2015 at 01:52:11PM +, Nathan Wagner wrote:
> On Mon, Oct 26, 2015 at 01:58:52PM -0400, Robert Haas wrote:
> > Aw, you're no fun.  select '1 fortnight'::interval => '14 days'
> > would be cool.
> 
> Patch attached...
> 
> :)

I'd argue for a back-patch all the way to 9.1, as the lack of
fortnight support is clearly a bug.

Cheers,
David.
-- 
David Fetter  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] fortnight interval support

[Merlin Moncure]

On Tue, Oct 27, 2015 at 8:52 AM, Nathan Wagner  wrote:
> On Mon, Oct 26, 2015 at 01:58:52PM -0400, Robert Haas wrote:
>> Aw, you're no fun.  select '1 fortnight'::interval => '14 days' would be 
>> cool.
>
> Patch attached...
>
> :)

This is very cool (you are 100% certain there are no performance
impacts on current cases, right?)!   :-)

merlin


-- 
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: Implement failover on libpq connect level.

[Josh Berkus]

On 10/27/2015 01:42 AM, Shulgin, Oleksandr wrote:
> On Mon, Oct 26, 2015 at 10:02 PM, Christopher Browne  > wrote:
> 
> 
> On 26 October 2015 at 16:25, Peter Eisentraut  > wrote:
> 
> On 10/14/15 6:41 AM, Victor Wagner wrote:
> > 1. It is allowed to specify several hosts in the connect string, 
> either
> > in URL-style (separated by comma) or in param=value form (several 
> host
> > parameters).
> 
> I'm not fond of having URLs that are not valid URLs according to the
> applicable standards.  Because then they can't be parsed or
> composed by
> standard libraries.
> 
> Also, this assumes that all the components other than host and
> port are
> the same.  Earlier there was a discussion about why the ports
> would ever
> need to be different.  Well, why can't the database names be
> different?
>  I could have use for that.
> 
> I think you should just accept multiple URLs.
> 
> 
> I'd give a "+1" on this...
> 
> As an area of new behaviour, I don't see a big problem with declining to
> support every wee bit of libpq configuration, and instead requiring the
> use of URLs.
> 
> Trying to put "multiplicities" into each parameter (and then considering
> it at the pg_service level, too) is WAY more complicated, and for a
> feature where it seems to me that it is pretty reasonable to have a
> series of fully qualified URLs.
> 
> Specifying several URLs should be easier to understand, easier to
> test, easier to code, and easier to keep from blowing up badly.
> 
> 
> Setting aside all other concerns, have a +1 from me on that too.

I'm good with this.  +1


-- 
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] [DOCS] max_worker_processes on the standby

[Alvaro Herrera]

Alvaro Herrera wrote:
> Robert Haas wrote:
> > On Sat, Oct 17, 2015 at 5:37 PM, Petr Jelinek  wrote:
> > > I agree with that sentiment.
> > >
> > > Attached patch adds variable to the shmem which is used for module
> > > activation tracking - set to true in ActiveCommitTs() and false in
> > > DeactivateCommitTs(). All the checks inside the commit_ts code were 
> > > changed
> > > to use this new variable. I also removed the static variable Alvaro added 
> > > in
> > > previous commit because it's not needed anymore.
> > 
> > That sounds good to me.  On a quick read-through it looks OK too.
> 
> A revised version is attached.

Pushed.

-- 
Á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] Rework the way multixact truncations work

[Josh Berkus]

On 10/27/2015 07:44 AM, Andres Freund wrote:
>> Unlinking old pg_clog files is strictly an optimization.  If you were to
>> > comment out every unlink() call in slru.c, the only ill effect on CLOG is 
>> > the
>> > waste of disk space.  Is the same true of MultiXact?
> Well, multixacts are a lot larger than the other SLRUs, I think that
> makes some sort of difference.

And by "a lot larger" we're talking like 50X to 100X.  I regularly see
pg_multixact directories larger than 1GB.

-- 
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] fortnight interval support

[Nathan Wagner]

On Tue, Oct 27, 2015 at 01:52:11PM +, Nathan Wagner wrote:
> On Mon, Oct 26, 2015 at 01:58:52PM -0400, Robert Haas wrote:
> > Aw, you're no fun.  select '1 fortnight'::interval => '14 days' would be 
> > cool.
> 
> Patch attached...

This isn't necessarily bad, but I observe that it would be difficult or
impossible to add fortnight support to intervals as an extension rather
than by modifying the scanner tables that the interval parser uses at
original compile time.  You might be able to override symbols with a C
extension, but parts of the parser are static (in the C sense), so you'd
need to override and duplicate a lot of the existing functions.  Of
course, a hookable interval parser is absurd in the first place.

-- 
nw


-- 
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] fortnight interval support

[Nathan Wagner]

On Tue, Oct 27, 2015 at 12:04:55PM -0500, Merlin Moncure wrote:
> On Tue, Oct 27, 2015 at 8:52 AM, Nathan Wagner  wrote:
> > On Mon, Oct 26, 2015 at 01:58:52PM -0400, Robert Haas wrote:
> >> Aw, you're no fun.  select '1 fortnight'::interval => '14 days' would be 
> >> cool.
> >
> > Patch attached...

> This is very cool (you are 100% certain there are no performance
> impacts on current cases, right?)!   :-)

It passed the regression test.  It must be perfect :)

-- 
nw


-- 
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] Limit GIST_MAX_SPLIT_PAGES to XLR_MAX_BLOCK_ID

[Gurjeet Singh]

(adding Heikki, since that macro was touched by his commit 04e298b8)

Does my previous description of the problem make sense, or am I fretting
over something that's a non-issue.

Best regards,


On Sun, Sep 20, 2015 at 7:03 PM, Gurjeet Singh  wrote:

> Gin code respects the XLR_MAX_BLOCK_ID when
> calling XLogEnsureRecordSpace(). But it appears that the Gist code does not
> try to limit its block-id consumption to XLR_MAX_BLOCK_ID.
>
> The GIST_MAX_SPLIT_PAGES is hard-coded at 75, but XLogEnsureRecordSpace()
> would reject a request of more than 32 (XLR_MAX_BLOCK_ID).
>
> The attached patch redefines GIST_MAX_SPLIT_PAGES so that in case of a
> split, gistplacetopage() now throws an error when the block-ids needed
> exceed 32.
>
> I have used Min(75, XLR_MAX_BLOCK_ID) as the macro expansion, but I
> believe it can be set to plain XLR_MAX_BLOCK_ID.
>
>
> --
> Gurjeet Singh http://gurjeet.singh.im/
>



-- 
Gurjeet Singh http://gurjeet.singh.im/

[HACKERS] quieting DEBUG3

[Robert Haas]

It's been my observation that setting client_min_messages or
log_min_messages to DEBUG3 is almost invariably impractical, because
every SQL statement spews about SIX messages about the transaction
state:

rhaas=# select 1;
DEBUG:  StartTransactionCommand
DEBUG:  StartTransaction
DEBUG:  name: unnamed; blockState:   DEFAULT; state: INPROGR,
xid/subid/cid: 0/1/0, nestlvl: 1, children:
DEBUG:  CommitTransactionCommand
DEBUG:  CommitTransaction
DEBUG:  name: unnamed; blockState:   STARTED; state: INPROGR,
xid/subid/cid: 0/1/0, nestlvl: 1, children:
 ?column?
--
1
(1 row)

I think this is excessive.  I will grant you that DEBUG3 is cranking
it up pretty high, but I notice that DEBUG2 produces no messages at
all in the same situation, and DEBUG5 doesn't produce any more than
DEBUG3.  The practical effect of this is that a logging level of
DEBUG3 or higher is impractical both on production systems (because
the logs will fill up) and for testing purposes (because sorting
through garbage of the type shown above is too much work).  Here's
what I propose:

1. Remove the StartTransactionCommand and CommitTransactionCommand
ereport calls from start_xact_command() and finish_xact_command.  I
can't think of any way that these calls can justify themselves at any
log level.  They're going to happen on every single statement.
They're just a constant string with no potentially-useful detail
information.  And they're largely redundant with the other messages.

2. Modify xact.c to reduce the log level of the remaining log messages
that appear in the above output from DEBUG3 to DEBUG5.

3. Refactor the logging in xact.c to be less chatty.
ShowTransactionState currently prints its string argument as a
separate message, and then recurses to dump one line per
TransactionState entry.  Instead, let's use it as a prefix for each of
the messages that follows.  Those messages are already quite long, but
if they only appear at DEBUG5 it doesn't matter that much, and we can
also easily make them shorter without losing information.  In
particular, I suggest that we:

- omit "name: unnamed; " from each line and instead only display the
name if there is one
- omit "children: " when no children exist
- print the blockstate using %s instead of %13s, to avoid adding
padding whitespace
- display the nest level in parentheses after the string prefix
instead of printing nestlvl: %d

So instead of this:

DEBUG:  StartTransaction
DEBUG:  name: unnamed; blockState:   DEFAULT; state: INPROGR,
xid/subid/cid: 0/1/0, nestlvl: 1, children:

We would get this:

DEBUG:  StartTransaction(1): blockState: DEFAULT; state: INPROGR,
xid/subid/cid: 0/1/0

...and only at DEBUG5.

Thoughts?

-- 
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] fortnight interval support

[Gavin Flower]

On 28/10/15 02:52, Nathan Wagner wrote:

On Mon, Oct 26, 2015 at 01:58:52PM -0400, Robert Haas wrote:

Aw, you're no fun.  select '1 fortnight'::interval => '14 days' would be cool.

Patch attached...

:)


[...]

You trying to get PostgreSQL banned in France???  :-)

When I was learning French many years ago, I was told that the French 
consider their fortnight to be 15 days!!!


see:
https://nz.answers.yahoo.com/question/index?qid=20100920093722AAc54Xo
https://en.wikipedia.org/wiki/Fortnight
http://www.wordsense.eu/fortnight


Cheers,
Gavin



--
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] fortnight interval support

[Nathan Wagner]

On Wed, Oct 28, 2015 at 08:17:25AM +1300, Gavin Flower wrote:
> You trying to get PostgreSQL banned in France???  :-)
> 
> When I was learning French many years ago, I was told that the French
> consider their fortnight to be 15 days!!!

What, it's a "fortnight", not a "quinzaine".

You have no idea how hard it was to resist updating the patch...

-- 
nw


-- 
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] Foreign join pushdown vs EvalPlanQual

[Robert Haas]

On Tue, Oct 20, 2015 at 12:39 PM, Etsuro Fujita
 wrote:
> Sorry, my explanation was not correct.  (Needed to take in caffeine.) What
> I'm concerned about is the following:
>
> SELECT * FROM localtab JOIN (ft1 LEFT JOIN ft2 ON ft1.x = ft2.x) ON
> localtab.id = ft1.id FOR UPDATE OF ft1
>
> LockRows
> -> Nested Loop
>  Join Filter: (localtab.id = ft1.id)
>  -> Seq Scan on localtab
>  -> Foreign Scan on 
>   Remote SQL: SELECT * FROM ft1 LEFT JOIN ft2 WHERE ft1.x = ft2.x
> FOR UPDATE OF ft1
>
> Assume that ft1 performs late row locking.

If the SQL includes "FOR UPDATE of ft1", then it clearly performs
early row locking.  I assume you meant to omit that.

> If an EPQ recheck was invoked
> due to a concurrent transaction on the remote server that changed only the
> value x of the ft1 tuple previously retrieved, then we would have to
> generate a fake ft1/ft2-join tuple with nulls for ft2. (Assume that the ft2
> tuple previously retrieved was not a null tuple.) However, I'm not sure how
> we can do that in ForeignRecheck; we can't know for example, which one is
> outer and which one is inner, without an alternative local join execution
> plan.  Maybe I'm missing something, though.

I would expect it to issue a new query like: SELECT * FROM ft1 LEFT
JOIN ft2 WHERE ft1.x = ft2.x AND ft1.tid = $0 AND ft2.tid = $1.

This should be significantly more efficient than fetching the base
rows from each of two tables with two separate queries.

-- 
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] remaining open items

[Alvaro Herrera]

Ian Barwick wrote:
> On 17/10/15 04:31, Alvaro Herrera wrote:
> > Robert Haas wrote:
> >>> The other item on me is the documentation patch by Emre Hasegeli for
> >>> usage of the inclusion opclass framework in BRIN.  I think it needs some
> >>> slight revision by some native English speaker and I'm not completely in
> >>> love with the proposed third column in the table it adds, but otherwise
> >>> is factually correct as far as I can tell.
> >>
> >> I'm not clear whether you are asking for help with this, or ...?
> > 
> > I enlisted the help of Ian Barwick for this one.
> 
> Revised version of Emre's patch attached, apologies for the delay.

Great, thanks, pushed.  I only changed this:

> !   strategies are embedded in the support procedures's source code.
> --- 535,541 
> !   strategies are embedded in the support procedure's source code.

to procedures' which is the right possessive form, I think (I want
"procedures" to remain in the plural form).

-- 
Á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] Parallel Seq Scan

[Robert Haas]

On Sat, Oct 24, 2015 at 6:31 PM, Noah Misch  wrote:
> On Sat, Oct 24, 2015 at 07:49:07AM -0400, Robert Haas wrote:
>> On Fri, Oct 23, 2015 at 9:38 PM, Noah Misch  wrote:
>> > Since that specification permits ParamListInfo consumers to ignore 
>> > paramMask,
>> > the plpgsql_param_fetch() change from copy-paramlistinfo-fixes.patch is 
>> > still
>> > formally required.
>>
>> So why am I not just doing that, then?  Seems a lot more surgical.
>
> do $$
> declare
> param_unused text := repeat('a', 100 * 1024 * 1024);
> param_used oid := 403;
> begin
> perform count(*) from pg_am where oid = param_used;
> end
> $$;
>
> I expect that if you were to inspect the EstimateParamListSpace() return
> values when executing that, you would find that it serializes the irrelevant
> 100 MiB datum.  No possible logic in plpgsql_param_fetch() could stop that
> from happening, because copyParamList() and SerializeParamList() call the
> paramFetch hook only for dynamic parameters.  Cursors faced the same problem,
> which is the raison d'être for setup_unshared_param_list().

Well, OK.  That's not strictly a correctness issue, but here's an
updated patch along the lines you suggested.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From 50895be5cdbb0fda41535be23700e5112585e1e3 Mon Sep 17 00:00:00 2001
From: Robert Haas 
Date: Thu, 22 Oct 2015 23:56:51 -0400
Subject: [PATCH 6/6] Fix problems with ParamListInfo serialization mechanism.

Commit d1b7c1ffe72e86932b5395f29e006c3f503bc53d introduced a mechanism
for serializing a ParamListInfo structure to be passed to a parallel
worker.  However, this mechanism failed to handle external expanded
values, as pointed out by Noah Misch.  Repair.

Moreover, plpgsql_param_fetch requires adjustment because the
serialization mechanism needs it to skip evaluating unused parameters
just as we would do when it is called from copyParamList, but params
== estate->paramLI in that case.  To fix, make the bms_is_member test
in that function unconditional.

Finally, have setup_param_list set a new ParamListInfo field,
paramMask, to the parameters actually used in the expression, so that
we don't try to fetch those that are not needed when serializing a
parameter list.  This isn't necessary for performance, but it makes
the performance of the parallel executor code comparable to what we
do for cases involving cursors.
---
 src/backend/commands/prepare.c   |  1 +
 src/backend/executor/functions.c |  1 +
 src/backend/executor/spi.c   |  1 +
 src/backend/nodes/params.c   | 54 
 src/backend/tcop/postgres.c  |  1 +
 src/backend/utils/adt/datum.c| 16 
 src/include/nodes/params.h   |  4 ++-
 src/pl/plpgsql/src/pl_exec.c | 40 -
 8 files changed, 89 insertions(+), 29 deletions(-)

diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index fb33d30..0d4aa69 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -392,6 +392,7 @@ EvaluateParams(PreparedStatement *pstmt, List *params,
 	paramLI->parserSetup = NULL;
 	paramLI->parserSetupArg = NULL;
 	paramLI->numParams = num_params;
+	paramLI->paramMask = NULL;
 
 	i = 0;
 	foreach(l, exprstates)
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 812a610..0919c04 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -910,6 +910,7 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
 			paramLI->parserSetup = NULL;
 			paramLI->parserSetupArg = NULL;
 			paramLI->numParams = nargs;
+			paramLI->paramMask = NULL;
 			fcache->paramLI = paramLI;
 		}
 		else
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 300401e..13ddb8f 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2330,6 +2330,7 @@ _SPI_convert_params(int nargs, Oid *argtypes,
 		paramLI->parserSetup = NULL;
 		paramLI->parserSetupArg = NULL;
 		paramLI->numParams = nargs;
+		paramLI->paramMask = NULL;
 
 		for (i = 0; i < nargs; i++)
 		{
diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c
index d093263..0351774 100644
--- a/src/backend/nodes/params.c
+++ b/src/backend/nodes/params.c
@@ -15,6 +15,7 @@
 
 #include "postgres.h"
 
+#include "nodes/bitmapset.h"
 #include "nodes/params.h"
 #include "storage/shmem.h"
 #include "utils/datum.h"
@@ -50,6 +51,7 @@ copyParamList(ParamListInfo from)
 	retval->parserSetup = NULL;
 	retval->parserSetupArg = NULL;
 	retval->numParams = from->numParams;
+	retval->paramMask = NULL;
 
 	for (i = 0; i < from->numParams; i++)
 	{
@@ -58,6 +60,17 @@ copyParamList(ParamListInfo from)
 		int16		typLen;
 		bool		typByVal;
 
+		/* Ignore parameters we don't need, to save cycles and space. */
+		if (retval->paramMask != NULL &&
+			!bms_is_member(i, retval->paramMask))

Re: [HACKERS] quieting DEBUG3

[Craig Ringer]

On 28 October 2015 at 04:52, Robert Haas  wrote:

> Thoughts?

I agree with you that DEBUG3 is currently impractical. So much so that
when testing/debugging I often change log severities of messages of
interest so they'll show up at DEBUG2 instead, rather than running in
DEBUG3.

Simplifying the StartTransaction log to a single message line is
clearly sensible.

I tend to see a lot of:

DEBUG:  StartTransactionCommand
DEBUG:  StartTransaction
DEBUG:  name: unnamed; blockState:   DEFAULT; state: INPROGR,
xid/subid/cid: 0/1/0, nestlvl: 1, children:
DEBUG:  ProcessUtility
DEBUG:  CommitTransactionCommand
DEBUG:  StartTransactionCommand
DEBUG:  ProcessQuery
DEBUG:  CommitTransactionCommand
DEBUG:  StartTransactionCommand
DEBUG:  ProcessQuery
DEBUG:  CommitTransactionCommand
DEBUG:  StartTransactionCommand
DEBUG:  ProcessUtility
DEBUG:  CommitTransactionCommand
DEBUG:  CommitTransaction
DEBUG:  name: unnamed; blockState:   END; state: INPROGR,
xid/subid/cid: 3778/1/2, nestlvl: 1, children:

It's pretty spammy, and the StartTransactionCommand /
CommitTransactionCommand entries don't seem to add any useful
information.

I'm not so sure about bumping transaction tracing down to DEBUG5,
given that transaction control is pretty important, and can be an
issue when chasing problems with cache accesses, etc. There's a lot of
other output, too, and once simplified to one line I think it's
reasonable to retain at DEBUG3.

I'd be pretty happy to get:


DEBUG:  StartTransaction(1): blockState: DEFAULT; state: INPROGR,
xid/subid/cid: 0/1/0
DEBUG:  ProcessUtility
DEBUG:  ProcessQuery
DEBUG:  ProcessQuery
DEBUG:  ProcessUtility
DEBUG:  CommitTransaction(1): blockState: END; state: INPROGR,
xid/subid/cid: 3778/1/2

instead.

I think it'd be helpful to define some level of policy about what the
debug levels are intended for, so there's some guidance on what level
to emit messages on rather than playing "pick a number". But that
shouldn't hold up the simplifications you propose here, except maybe
the DEBUG3 to DEBUG5 change.

-- 
 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] [COMMITTERS] pgsql: Cleanup commit timestamp module activaction, again

[Alvaro Herrera]

Tom Lane wrote:
> Robert Haas  writes:
> > On Tue, Oct 27, 2015 at 7:07 PM, Alvaro Herrera  
> > wrote:
> >> Cleanup commit timestamp module activaction, again
> 
> > The buildfarm seems to be very unhappy right now, and it looks as if
> > this commit is to blame.
> 
> I think it's just failure to update an expected-file, but will leave it
> to Alvaro.

I don't understand.  It passed make check fine for me on both branches.
Will fix in a jiffy.

-- 
Á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] [COMMITTERS] pgsql: Cleanup commit timestamp module activaction, again

[Tom Lane]

Robert Haas  writes:
> On Tue, Oct 27, 2015 at 7:07 PM, Alvaro Herrera  
> wrote:
>> Cleanup commit timestamp module activaction, again

> The buildfarm seems to be very unhappy right now, and it looks as if
> this commit is to blame.

I think it's just failure to update an expected-file, but will leave it
to Alvaro.

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] [COMMITTERS] pgsql: Cleanup commit timestamp module activaction, again

[Robert Haas]

On Tue, Oct 27, 2015 at 7:07 PM, Alvaro Herrera  wrote:
> Cleanup commit timestamp module activaction, again
>
> Further tweak commit_ts.c so that on a standby the state is completely
> consistent with what that in the master, rather than behaving
> differently in the cases that the settings differ.  Now in standby and
> master the module should always be active or inactive in lockstep.
>
> Author: Petr Jelínek, with some further tweaks by Álvaro Herrera.
>
> Backpatch to 9.5, where commit timestamps were introduced.

The buildfarm seems to be very unhappy right now, and it looks as if
this commit is to blame.

-- 
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] [COMMITTERS] pgsql: Cleanup commit timestamp module activaction, again

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane wrote:
>> I think it's just failure to update an expected-file, but will leave it
>> to Alvaro.

> I don't understand.  It passed make check fine for me on both branches.
> Will fix in a jiffy.

There are two variant expected files for that module --- did you fix
both?

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] quieting DEBUG3

[Tom Lane]

Craig Ringer  writes:
> I think it'd be helpful to define some level of policy about what the
> debug levels are intended for, so there's some guidance on what level
> to emit messages on rather than playing "pick a number".

+1 ... I doubt anyone has ever looked at that in a holistic way.

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] Freeze avoidance of very large table.

[Amit Kapila]

On Sat, Oct 24, 2015 at 2:24 PM, Masahiko Sawada 
wrote:
>
> On Sat, Oct 24, 2015 at 10:59 AM, Amit Kapila 
wrote:
> >
> > I think we can display information about relallfrozen it in
pg_stat_*_tables
> > as suggested by you.  It doesn't make much sense to keep it in pg_class
> > unless we have some usecase for the same.
> >
>
> I'm thinking a bit about implementing the read-only table that is
> restricted to update/delete and is ensured that whole table is frozen,
> if this feature is committed.
> The value of relallfrozen might be useful for such feature.
>

If we need this for read-only table feature, then better lets add that
after discussing the design of that feature.  It doesn't seem to be
advisable to have an extra field in system table which we might
need in yet not completely-discussed feature.

Review Comments:
---
1.
  /*
- * Find buffer to insert this tuple into.  If the page is all visible,
- * this will also pin
the requisite visibility map page.
+ * Find buffer to insert this tuple into.  If the page is all
visible
+ * or all frozen, this will also pin the requisite visibility map and
+ * frozen map page.

 */
  buffer = RelationGetBufferForTuple(relation, heaptup->t_len,

  InvalidBuffer, options, bistate,


I think it is sufficient to say in the end 'visibility map page'.
Let's not include 'frozen map page'.


2.
+ * corresponding page has been completely frozen, so the visibility map is
also
+ * used for anti-wraparound
vacuum, even if freezing tuples is required.

/all tuple/all tuples
/freezing tuples/freezing of tuples

3.
- * Are all tuples on heapBlk visible to all, according to the visibility
map?
+ * Are all tuples on heapBlk
visible or frozen to all, according to the visibility map?

I think it is better to modify the above statement as:
Are all tuples on heapBlk visible to all or are marked as frozen, according
to the visibility map?

4.
+ * releasing *buf after it's done testing and setting bits, and must set
flags
+ * which indicates what flag
we want to test.

Here are you talking about the flags passed to visibilitymap_set(), if
yes, then above comment is not clear, how about:

and must pass flags
for which it needs to check the value in visibility map.

5.
+ * both how many pages we skipped according to all-frozen bit of visibility
+ * map and how many
pages we freeze page, so we can update relfrozenxid if

In above sentence word 'page' after freeze sounds redundant.
/we freeze page/we freeze

Another suggestion:
/sum of them/sum of two

6.
+ * This block is at least all-visible according to visibility map.
+
 * We check whehter this block is all-frozen or not, to skip to

whether is mis-spelled

7.
+ * If we froze any tuples or any tuples are already frozen,
+ * mark the buffer
dirty, and write a WAL record recording the changes.

Here, I think WAL record is written only when we mark some
tuple/'s as frozen not if we they are already frozen,
so in that regard, I think above comment is wrong.

8.
+ /*
+ * We cant't allow upgrading with link mode between 9.5 or before and 9.6
or later,
+ *
because the format of visibility map has been changed on version 9.6.
+ */


a. /cant't/can't
b. changed on version 9.6/changed in version 9.6
b. Won't such a change needs to be updated in pg_upgrade
documentation (Notes Section)?

9.
@@ -180,6 +181,13 @@ transfer_single_new_db(pageCnvCtx *pageConverter,

new_cluster.controldata.cat_ver >= VISIBILITY_MAP_CRASHSAFE_CAT_VER)
  vm_crashsafe_match = false;

+
/*
+ * Do we need to rewrite visibilitymap?
+ */
+ if (old_cluster.controldata.cat_ver <
VISIBILITY_MAP_FROZEN_BIT_CAT_VER &&
+ new_cluster.controldata.cat_ver >=
VISIBILITY_MAP_FROZEN_BIT_CAT_VER)
+ vm_rewrite_needed = true;

..

@@ -276,7 +285,15 @@ transfer_relfile(pageCnvCtx *pageConverter,
FileNameMap *map,
  {

pg_log(PG_VERBOSE, "copying \"%s\" to \"%s\"\n", old_file, new_file);

- if ((msg =
copyAndUpdateFile(pageConverter, old_file, new_file, true)) != NULL)
+ /*
+
 * Do we need to rewrite visibilitymap?
+ */
+ if (strcmp
(type_suffix, "_vm") == 0 &&
+ old_cluster.controldata.cat_ver <
VISIBILITY_MAP_FROZEN_BIT_CAT_VER &&
+ new_cluster.controldata.cat_ver >=
VISIBILITY_MAP_FROZEN_BIT_CAT_VER)
+ rewrite_vm = true;

Instead of doing re-check in transfer_relfile(), I think it is better
to pass an additional parameter in this function.

10.
You have mentioned up-thread that, you have changed the patch so that
PageClearAllVisible clear both bits, can you please point me to this
change?
Basically after applying the patch, I see below code in bufpage.h:
#define PageClearAllVisible(page) \
(((PageHeader) (page))->pd_flags &= ~PD_ALL_VISIBLE)

Don't we need to clear the PD_ALL_FROZEN separately?


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

Re: [HACKERS] Disabling START TRANSACTION for a SuperUser

[Muthiah Rajan]

Hello Kevin,

This may be a trivial thing But what do you mean by shops? I actually
can't get it :-)
On 27-Oct-2015 7:37 PM, "Kevin Grittner"  wrote:

> On Tuesday, October 27, 2015 8:52 AM, Craig Ringer 
> wrote:
> > On 27 October 2015 at 21:19, rajan  wrote:
>
> >> Thanks for your response. Seems like the workaround is difficult.
> >>
> >> I am trying to understand
> >> "
> >> ExecutorStart_hook and ProcessUtility_hook
>
> > Doing what you want will require being willing to spend a fair bit of
> > time becoming familiar with PostgreSQL's innards, writing extensions,
> > etc. It's not a simple "download, compile, run" process. You will need
> > to be confident with C programming and reading source code.
>
> > If this is going way too deep, perhaps you should post to
> > pgsql-general with a description of the underlying problem you are
> > trying to solve, i.e. *why* you want to be able to have a superuser
> > who can alter users but can't select, etc. What's the problem you're
> > trying to solve with this?
>
> This is a question I have seen before, as well as slight variations
> on it related to transaction isolation level.  Right now you can
> implement a read-only user by granting only SELECT rights to tables
> and also by setting the default_transaction_read_only = on.  The
> problem is that the latter is essentially just a suggestion, not an
> order.  I actually don't think it's as big a problem with read-only
> users, since that can still be accomplished (with enough work) by
> using the GRANT/REVOKE commands.  (Think how much faster and easier
> it could be if there is a role that allows the appropriate set of
> SELECTs but also allows some DML -- just set a read-only rule for
> the user and the existing role could work.)
>
> It is more problematic where a shop wants to use serializable
> transactions to ensure data integrity.  The only way to prevent
> someone from subverting the business rules is to code a lot of
> triggers on a lot of objects that throw an error if the isolation
> level is wrong.  It would be a boon to big shops if they could
> declare (preferably with the option to set it at a role level) that
> specific default_transaction_* settings could not be overridden.
>
> --
> Kevin Grittner
> EDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] [COMMITTERS] pgsql: Cleanup commit timestamp module activaction, again

[Alvaro Herrera]

Tom Lane wrote:
> Alvaro Herrera  writes:
> > Tom Lane wrote:
> >> I think it's just failure to update an expected-file, but will leave it
> >> to Alvaro.
> 
> > I don't understand.  It passed make check fine for me on both branches.
> > Will fix in a jiffy.
> 
> There are two variant expected files for that module --- did you fix
> both?

Yeah, this was the problem: make installcheck almost always exercises
the case with the setting disabled, which make check doesn't test.

-- 
Á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] Disabling START TRANSACTION for a SuperUser

[Muthiah Rajan]

Thanks Craig,

There are a lot of details and its overwhelming :-) Let me digest and
will post for any help
On 27-Oct-2015 7:21 PM, "Craig Ringer"  wrote:

> On 27 October 2015 at 21:19, rajan <[hidden email]
> > wrote:
> > Hey Craig,
> >
> > Thanks for your response. Seems like the workaround is difficult.
> >
> > I am trying to understand
> > "
> > ExecutorStart_hook and ProcessUtility_hook
>
> Doing what you want will require being willing to spend a fair bit of
> time becoming familiar with PostgreSQL's innards, writing extensions,
> etc. It's not a simple "download, compile, run" process. You will need
> to be confident with C programming and reading source code.
>
> Here's some code that filters allowable commands. It doesn't care
> which user id is used, but it's pretty simple to add a check to only
> run the filter when a particular user ID is the active user. This
> won't do what you want, but serves as a rough example of how you can
> filter statements based on the parsed statement data:
>
> https://github.com/2ndQuadrant/bdr/blob/bdr-plugin/next/bdr_commandfilter.c
>
> and also:
>
> http://www.postgresql.org/docs/current/static/xfunc-c.html
> http://www.postgresql.org/docs/current/static/extend-extensions.html
> http://www.postgresql.org/docs/current/static/extend-pgxs.html
>
> Note that BDR's command filter doesn't do anything to
> insert/update/delete/select. For that you'd *also* need an
> ExecutorStart_hook or similar.
>
> If this is going way too deep, perhaps you should post to
> pgsql-general with a description of the underlying problem you are
> trying to solve, i.e. *why* you want to be able to have a superuser
> who can alter users but can't select, etc. What's the problem you're
> trying to solve with this?
>
> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list ([hidden email]
> )
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
> --
> If you reply to this email, your message will be added to the discussion
> below:
>
> http://postgresql.nabble.com/Disabling-START-TRANSACTION-for-a-SuperUser-tp5871630p5871647.html
> To unsubscribe from Disabling START TRANSACTION for a SuperUser, click
> here
> 
> .
> NAML
> 
>

Re: [HACKERS] remaining open items

[Alvaro Herrera]

Craig Ringer wrote:
> On 14 October 2015 at 09:57, Robert Haas  wrote:
> > We've got a few open items remaining at
> > https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items - we should
> > try to get rid of them.
> 
> I'd like to add, rather than remove, one. A bug in replication origins
> support. It comes with a simple fix.

Please edit the wiki to get it listed.

-- 
Á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] Disabling START TRANSACTION for a SuperUser

[Craig Ringer]

On 27 October 2015 at 21:19, rajan  wrote:
> Hey Craig,
>
> Thanks for your response. Seems like the workaround is difficult.
>
> I am trying to understand
> "
> ExecutorStart_hook and ProcessUtility_hook

Doing what you want will require being willing to spend a fair bit of
time becoming familiar with PostgreSQL's innards, writing extensions,
etc. It's not a simple "download, compile, run" process. You will need
to be confident with C programming and reading source code.

Here's some code that filters allowable commands. It doesn't care
which user id is used, but it's pretty simple to add a check to only
run the filter when a particular user ID is the active user. This
won't do what you want, but serves as a rough example of how you can
filter statements based on the parsed statement data:

https://github.com/2ndQuadrant/bdr/blob/bdr-plugin/next/bdr_commandfilter.c

and also:

http://www.postgresql.org/docs/current/static/xfunc-c.html
http://www.postgresql.org/docs/current/static/extend-extensions.html
http://www.postgresql.org/docs/current/static/extend-pgxs.html

Note that BDR's command filter doesn't do anything to
insert/update/delete/select. For that you'd *also* need an
ExecutorStart_hook or similar.

If this is going way too deep, perhaps you should post to
pgsql-general with a description of the underlying problem you are
trying to solve, i.e. *why* you want to be able to have a superuser
who can alter users but can't select, etc. What's the problem you're
trying to solve with this?

-- 
 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

[HACKERS] fortnight interval support

[Nathan Wagner]

On Mon, Oct 26, 2015 at 01:58:52PM -0400, Robert Haas wrote:
> Aw, you're no fun.  select '1 fortnight'::interval => '14 days' would be cool.

Patch attached...

:)

% psql -p 5433 -d template1 -h localhost
psql (9.4.5, server 9.6devel)
WARNING: psql major version 9.4, server major version 9.6.
 Some psql features might not work.
Type "help" for help.

template1=# select current_date;
date

 2015-10-27
(1 row)

template1=# select '1 fortnight'::interval;
 interval 
--
 14 days
(1 row)

template1=# select current_date + '1 fortnight'::interval;
  ?column?   
-
 2015-11-10 00:00:00
(1 row)

template1=# select current_date + '1.3 fortnight'::interval;
  ?column?   
-
 2015-11-14 04:48:00
(1 row)

template1=# select current_date + '1.3 fortnights'::interval;
  ?column?   
-
 2015-11-14 04:48:00
(1 row)

-- 
nw
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 926358e..2032fe0 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -186,6 +186,8 @@ static const datetkn deltatktbl[] = {
{DDECADE, UNITS, DTK_DECADE},   /* "decade" relative */
{"decades", UNITS, DTK_DECADE}, /* "decades" relative */
{"decs", UNITS, DTK_DECADE},/* "decades" relative */
+   {DFORTNIGHT, UNITS, DTK_FORTNIGHT}, /* "fortnights" relative */
+   {"fortnights", UNITS, DTK_FORTNIGHT}, /* "fortnights" relative */
{"h", UNITS, DTK_HOUR}, /* "hour" relative */
{DHOUR, UNITS, DTK_HOUR},   /* "hour" relative */
{"hours", UNITS, DTK_HOUR}, /* "hours" relative */
@@ -3281,6 +3283,12 @@ DecodeInterval(char **field, int *ftype, int nf, int 
range,
tmask = DTK_M(DAY);
break;
 
+   case DTK_FORTNIGHT:
+   tm->tm_mday += val * 14;
+   AdjustFractDays(fval, tm, fsec, 
14);
+   tmask = DTK_M(WEEK);
+   break;
+
case DTK_WEEK:
tm->tm_mday += val * 7;
AdjustFractDays(fval, tm, fsec, 
7);
diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index e9a1ece..3641292 100644
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -52,6 +52,7 @@ struct tzEntry;
 #define DHOUR  "hour"
 #define DDAY   "day"
 #define DWEEK  "week"
+#define DFORTNIGHT "fortnight"
 #define DMONTH "month"
 #define DQUARTER   "quarter"
 #define DYEAR  "year"
@@ -181,6 +182,7 @@ struct tzEntry;
 #define DTK_TZ_MINUTE  35
 #define DTK_ISOYEAR36
 #define DTK_ISODOW 37
+#define DTK_FORTNIGHT  38
 
 
 /*
diff --git a/src/test/regress/expected/interval.out 
b/src/test/regress/expected/interval.out
index c873a99..7a72f2a 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -40,6 +40,12 @@ SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
  10 days 12:00:00
 (1 row)
 
+SELECT INTERVAL '1 fortnight' AS "Fourteen days";
+ Fourteen days 
+---
+ 14 days
+(1 row)
+
 SELECT INTERVAL '1.5 months' AS "One month 15 days";
  One month 15 days 
 ---
diff --git a/src/test/regress/sql/interval.sql 
b/src/test/regress/sql/interval.sql
index 789c3de..285266a 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -12,6 +12,7 @@ SELECT INTERVAL '-08:00' AS "Eight hours";
 SELECT INTERVAL '-1 +02:03' AS "22 hours ago...";
 SELECT INTERVAL '-1 days +02:03' AS "22 hours ago...";
 SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
+SELECT INTERVAL '1 fortnight' AS "Fourteen days";
 SELECT INTERVAL '1.5 months' AS "One month 15 days";
 SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
 

-- 
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] remaining open items

[Craig Ringer]

On 14 October 2015 at 09:57, Robert Haas  wrote:
> We've got a few open items remaining at
> https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items - we should
> try to get rid of them.

I'd like to add, rather than remove, one. A bug in replication origins
support. It comes with a simple fix.

See patch:

http://www.postgresql.org/message-id/camsr+yg3cwuxcsyxn1pxjy+zx8i6o71sgutegduzq+sbu4e...@mail.gmail.com

and thread start:

http://www.postgresql.org/message-id/CAMsr+YFhBJLp=qfsz3-j+0p1zlke8znxm2otycn20qrmx38...@mail.gmail.com





-- 
 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] Disabling START TRANSACTION for a SuperUser

[Kevin Grittner]

On Tuesday, October 27, 2015 8:52 AM, Craig Ringer  
wrote:
> On 27 October 2015 at 21:19, rajan  wrote:

>> Thanks for your response. Seems like the workaround is difficult.
>>
>> I am trying to understand
>> "
>> ExecutorStart_hook and ProcessUtility_hook

> Doing what you want will require being willing to spend a fair bit of
> time becoming familiar with PostgreSQL's innards, writing extensions,
> etc. It's not a simple "download, compile, run" process. You will need
> to be confident with C programming and reading source code.

> If this is going way too deep, perhaps you should post to
> pgsql-general with a description of the underlying problem you are
> trying to solve, i.e. *why* you want to be able to have a superuser
> who can alter users but can't select, etc. What's the problem you're
> trying to solve with this?

This is a question I have seen before, as well as slight variations
on it related to transaction isolation level.  Right now you can
implement a read-only user by granting only SELECT rights to tables
and also by setting the default_transaction_read_only = on.  The
problem is that the latter is essentially just a suggestion, not an
order.  I actually don't think it's as big a problem with read-only
users, since that can still be accomplished (with enough work) by
using the GRANT/REVOKE commands.  (Think how much faster and easier
it could be if there is a role that allows the appropriate set of
SELECTs but also allows some DML -- just set a read-only rule for
the user and the existing role could work.)

It is more problematic where a shop wants to use serializable
transactions to ensure data integrity.  The only way to prevent
someone from subverting the business rules is to code a lot of
triggers on a lot of objects that throw an error if the isolation
level is wrong.  It would be a boon to big shops if they could
declare (preferably with the option to set it at a role level) that
specific default_transaction_* settings could not be overridden.

--
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