Re: proposal: new polymorphic types - commontype and commontypearray

2019-01-26 Thread Pavel Stehule
>
>
>
>> My second problem with this proposal is that it simply ignores
>> the naming precedent of the existing polymorphic types.  We have
>> a convention that polymorphic types are named "any-something",
>> and I do not think we should just toss that overboard.  Moreover,
>> if we do end up needing "commonnonarray" or "commonenum", those
>> names are ugly, typo-prone, and unreasonably long.
>>
>
the convention "any-something" is joined with just currently implemented
families of polymorphic types. I propose new family, so I think so it
should not be named "any-"

Maybe we can use some form of typemod - but typemod is ignored for function
parameters - so it can be much more significant change

a alternative, probably very simple, but less power solution can be some
special flag for function parameters - at the end, it is similar to
previous solution.

I can imagine

create or replace function fx(p1 anyelement use_common_type, p2 anyelement,
...)
create or replace function fx2(p1 int, p2 int, variadic p3 anyarray
use_common_type)

or maybe

create or replace function fx(p1 anyelement, p2 anyelement ...) ...
language plpgsql options (use_common_type = true)

or we can drop it - on other thread you propose supported functions - can
be some function, that can preproces parameters - and can replace
polymorphic types by real types.

Comments, notes?

Pavel






> regards, tom lane
>>
>


Re: using expression syntax for partition bounds

2019-01-26 Thread Peter Eisentraut
On 25/01/2019 16:19, Tom Lane wrote:
> Peter Eisentraut  writes:
>> committed
> 
> Some of the buildfarm members are having sort-ordering problems
> with this.  Looks like you could work around it with different
> partition names (don't assume the relative sort order of
> letters and digits).

Fixed by light renaming.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: House style for DocBook documentation?

2019-01-26 Thread Peter Eisentraut
On 25/01/2019 15:37, Chapman Flack wrote:
>> External links already create footnotes in the PDF output.  Is that
>> different from what you are saying?
> That might, only, indicate that I was just thinking aloud in email and
> had not gone and checked in PDF output to see how the links were handled.
> 
> Yes, it could very possibly indicate that.
> 
> If they are already processed that way, does that mean the
> 
> o  Do not use text with  so the URL appears in printed output
> 
> in README.links should be considered obsolete, and removed even, and
> doc authors should feel free to put link text in  without
> hesitation?

I think it's obsolete, yes.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



[Patch] Log10 and hyperbolic functions for SQL:2016 compliance

2019-01-26 Thread Lætitia Avrot
Hello hackers,

In his blog post (What's new in SQL 2016)[
https://modern-sql.com/blog/2017-06/whats-new-in-sql-2016], Markus Winand
explained some of the changes added to SQL:2016. I spotted that Postgres
was behind other RDBMS on hyperbolic functions and log10 function.
The log10 function existed but under the name log().

The new functions can be called in a simple select statement :

select log10(100);
select sinh(0);
select cosh(0);
select tanh(0);

Even if Markus Winand had added hyperbolic functions in the paragraph
"Trigonometric and Logarithmic Functions", I didn't add hyperbolic function
with the trigonometric functions in the documentation, because hyperbolic
functions are not trigonometric functions.

I added regression tests for the new functions, but I didn't for log10
function, assuming that if log function worked, log10 will work too.

You'll find enclosed the first version of the patch that can build
successfully on my laptop against master. I'm open to any improvement.

Cheers,

Lætitia
-- 
*Think! Do you really need to print this email ? *
*There is no Planet B.*
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4930ec17f6..94e3fcd4b9 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -896,6 +896,19 @@
2
   
 
+  
+   
+
+ log10
+
+log10(dp or numeric)
+   
+   (same as input)
+   base 10 logarithm
+   log10(100.0)
+   2
+  
+
   
log(b numeric,
 x numeric)
@@ -1147,7 +1160,7 @@
   
 
   
-   Finally,  shows the
+shows the
available trigonometric functions.  All trigonometric functions
take arguments and return values of type double
precision.  Each of the trigonometric functions comes in
@@ -1311,8 +1324,65 @@

   
 
-  
+  
+   Finally,  shows the
+   available hyperbolic functions.
+  
 
+  
+Hyperbolic Functions
+
+
+ 
+  
+   Function
+   Return Type
+   Description
+   Example
+   Result
+  
+ 
+ 
+  
+   
+
+ sinh
+
+sinh(dp)
+   
+   dp
+   hyperbolic sine
+   sinh(0)
+   0
+  
+  
+   
+
+ cosh
+
+cosh(dp)
+   
+   dp
+   hyperbolic cosine
+   cosh(0)
+   1
+  
+  
+   
+
+ tanh
+
+tanh(dp)
+   
+   dp
+   hyperbolic tangent
+   tanh(0)
+   0
+  
+ 
+
+   
+  
 
   
String Functions and Operators
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index 117ded8d1d..c83eed99c6 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -2375,6 +2375,54 @@ radians(PG_FUNCTION_ARGS)
 	PG_RETURN_FLOAT8(float8_mul(arg1, RADIANS_PER_DEGREE));
 }
 
+/* == HYPERBOLIC FUNCTIONS == */
+
+/*
+ *		dsinh			- returns the hyperbolic sine of arg1
+ */
+Datum
+dsinh(PG_FUNCTION_ARGS)
+{
+	float8		arg1 = PG_GETARG_FLOAT8(0);
+	float8		result;
+
+	result = sinhf(arg1);
+
+	check_float8_val(result, isinf(arg1), true);
+	PG_RETURN_FLOAT8(result);
+}
+
+
+/*
+ *		dcosh			- returns the hyperbolic cosine of arg1
+ */
+Datum
+dcosh(PG_FUNCTION_ARGS)
+{
+	float8		arg1 = PG_GETARG_FLOAT8(0);
+	float8		result;
+
+	result = coshf(arg1);
+
+	check_float8_val(result, isinf(arg1), false);
+	PG_RETURN_FLOAT8(result);
+}
+
+/*
+ *		dtanh			- returns the hyperbolic tangent of arg1
+ */
+Datum
+dtanh(PG_FUNCTION_ARGS)
+{
+	float8		arg1 = PG_GETARG_FLOAT8(0);
+	float8		result;
+
+	errno = 0;
+	result = tanhf(arg1);
+
+	check_float8_val(result, false, true);
+	PG_RETURN_FLOAT8(result);
+}
 
 /*
  *		drandom		- returns a random number
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 3ecc2e12c3..ca6b09a13d 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -2617,6 +2617,9 @@
 { oid => '1340', descr => 'base 10 logarithm',
   proname => 'log', prorettype => 'float8', proargtypes => 'float8',
   prosrc => 'dlog10' },
+{ oid => '1364', descr => 'base 10 logarithm',
+  proname => 'log10', prorettype => 'float8', proargtypes => 'float8',
+  prosrc => 'dlog10' },
 { oid => '1341', descr => 'natural logarithm',
   proname => 'ln', prorettype => 'float8', proargtypes => 'float8',
   prosrc => 'dlog1' },
@@ -3283,6 +3286,17 @@
 { oid => '1610', descr => 'PI',
   proname => 'pi', prorettype => 'float8', proargtypes => '', prosrc => 'dpi' },
 
+{ oid => '2462', descr => 'hyperbolic sine',
+  proname => 'sinh', prorettype => 'float8', proargtypes => 'float8',
+  prosrc => 'dsinh' },
+{ oid => '2463', descr => 'hyperbolic cosine',
+  proname => 'cosh', prorettype => 'float8', proargtypes => 'float8',
+  prosrc => 'dcosh' },
+{ oid => '2464', descr => 'hyperbolic tangent',
+  proname => 'tanh', prorettype => 'float8', proargtypes => 'float8',
+  prosrc => 'dtanh' },
+
+
 { oid => '1618',
   p

Re: Log a sample of transactions

2019-01-26 Thread Adrien NAYRAT

On 1/23/19 3:12 AM, Kuroda, Hayato wrote:

Dear Adrien,


Hello Kuroda-san




 config.sgml 
You must document the behavior when users change the parameter during a 
transaction.
あやしい・・・


Agreed, I added a wording.

  
 postgres.c 

I give you three comments.


/* flag for logging statements in this transaction */

I think "a" or the plural form should be used instead of "this"


Fixed



* xact_is_sampled is left at the end of a transaction.
Should the parameter be set to false at the lowest layer of the transaction 
system?
I understand it is unnecessary for the functionality, but it have more symmetry.


Yes, it is not necessary. I wonder what is more important : keep some 
kind of symmetry or avoid unnecessary code (which can be source of mistake)?




* check_log_duration should be used only when postgres check the duration.
But I'm not sure a new function such as check_is_sampled is needed because A 
processing in new function will be as almost same as check_log_duration.


I agree, I asked myself the same question and I preferred to keep code 
simple.


Here is 4th patch.

Thanks!
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index b6f5822b84..29b64c51b3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5780,6 +5780,25 @@ local0.*/var/log/postgresql

   
 
+ 
+  log_transaction_sample_rate (real)
+  
+   log_transaction_sample_rate configuration parameter
+  
+  
+   
+
+ Set the fraction of transactions whose statements are logged. It applies
+ to each new transaction regardless of their duration. The default is
+ 0, meaning do not log statements from this transaction.
+ Setting this to 1 logs all statements for all transactions.
+ Logging can be disabled inside a transaction by setting this to 0.
+ log_transaction_sample_rate is helpful to track a
+ sample of transaction.
+
+   
+  
+
  
 
 
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 7c3a9c1e89..1a52c10c39 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1821,6 +1821,9 @@ StartTransaction(void)
 	s->state = TRANS_START;
 	s->transactionId = InvalidTransactionId;	/* until assigned */
 
+	/* Determine if we log statements in this transaction */
+	xact_is_sampled = random() <= log_xact_sample_rate * MAX_RANDOM_VALUE;
+
 	/*
 	 * initialize current transaction state fields
 	 *
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e773f20d9f..a11667834e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -100,7 +100,8 @@ int			max_stack_depth = 100;
 /* wait N seconds to allow attach from a debugger */
 int			PostAuthDelay = 0;
 
-
+/* flag for logging statements in a transaction */
+bool		xact_is_sampled = false;
 
 /* 
  *		private variables
@@ -2203,6 +2204,8 @@ check_log_statement(List *stmt_list)
  * check_log_duration
  *		Determine whether current command's duration should be logged.
  *		If log_statement_sample_rate < 1.0, log only a sample.
+ *		We also check if this statement in this transaction must be logged
+ *		(regardless of its duration).
  *
  * Returns:
  *		0 if no logging is needed
@@ -2218,7 +2221,8 @@ check_log_statement(List *stmt_list)
 int
 check_log_duration(char *msec_str, bool was_logged)
 {
-	if (log_duration || log_min_duration_statement >= 0)
+	if (log_duration || log_min_duration_statement >= 0 ||
+		(xact_is_sampled && log_xact_sample_rate > 0))
 	{
 		long		secs;
 		int			usecs;
@@ -2252,11 +2256,12 @@ check_log_duration(char *msec_str, bool was_logged)
 			(log_statement_sample_rate == 1 ||
 			 random() <= log_statement_sample_rate * MAX_RANDOM_VALUE);
 
-		if ((exceeded && in_sample) || log_duration)
+		if ((exceeded && in_sample) || log_duration ||
+			(xact_is_sampled && log_xact_sample_rate > 0))
 		{
 			snprintf(msec_str, 32, "%ld.%03d",
 	 secs * 1000 + msecs, usecs % 1000);
-			if (exceeded && !was_logged)
+			if ((exceeded || xact_is_sampled) && !was_logged)
 return 2;
 			else
 return 1;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c216ed0922..06a5e668aa 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -494,6 +494,7 @@ int			client_min_messages = NOTICE;
 int			log_min_duration_statement = -1;
 int			log_temp_files = -1;
 double		log_statement_sample_rate = 1.0;
+double		log_xact_sample_rate = 0;
 int			trace_recovery_messages = LOG;
 
 int			temp_file_limit = -1;
@@ -3347,13 +3348,25 @@ static struct config_real ConfigureNamesReal[] =
 		{"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN,
 			gettext_noop("Fraction of statements over log_min_duration_statement to log."),
 			gettext_noop("If you only want a sample, use a value between 0 (never "
-		 "log) and 1.0 (always log).")
+

Re: [PATCH] Allow UNLISTEN during recovery

2019-01-26 Thread Simon Riggs
On Sat, 26 Jan 2019 at 02:17, Tom Lane  wrote:

> Shay Rojansky  writes:
> > Thanks for insisting - I ended up setting up the environment and running
> > the tests, and discovering that some test-related changes were missing.
> > Here's a 3rd version of the patch. Hope this is now in good shape, let me
> > know if you think anything else needs to be done.
>
> Lotta work for a one-line code change, eh?  Pushed now.
>

Good decision, thanks.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: pg_basebackup, walreceiver and wal_sender_timeout

2019-01-26 Thread Nick B
On Sat, Jan 26, 2019 at 4:23 AM Michael Paquier  wrote:
> These are a bit unregular.  Which files are taking that long to
> complete while others are way faster?  It may be something that we
> could improve on the base backup side as there is no actual point in
> syncing segments while the backup is running and we could delay that
> at the end of the backup (if I recall that stuff correctly).

I don't have a good sample for these. One instance of this happening is below:

0.000125 fsync(7)  = 0 <0.016677>
0.39 fsync(7)  = 0 <0.05>
# 2048 writes for total of 16777216 bytes (16MB)
0.000618 write(7,
"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
8192) = 8192 <0.21>
0.78 fsync(8)  = 0 <57.609720>
57.609830 fsync(8)  = 0 <0.07>

Again, it is a problem with our network file system that we are still
investigating. I'm not sure this can be improved easily, since
pg_basebackup shares this code with walreceiver.

> The docs could be improved to describe that better..

I will look into that.

Regards,
Nick.



Re: Thread-unsafe coding in ecpg

2019-01-26 Thread Michael Meskes
> It looks to me like the reason that the ecpg tests went into an
> infinite
> loop is that compat_informix/test_informix.pgc has not considered the
> possibility of repeated statement failures:
> ...

Correct, this was missing a safeguard. 

> I know zip about ecpg coding practices, but wouldn't it be a better
> idea
> to break out of the loop on seeing an error?

I wonder if it would be better to make the test cases use the proper
whenever command instead. That would give us a slightly better
functionality testing I'd say.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL




Re: pg_basebackup, walreceiver and wal_sender_timeout

2019-01-26 Thread Magnus Hagander
On Sat, Jan 26, 2019 at 1:35 PM Nick B  wrote:

> On Sat, Jan 26, 2019 at 4:23 AM Michael Paquier 
> wrote:
> > These are a bit unregular.  Which files are taking that long to
> > complete while others are way faster?  It may be something that we
> > could improve on the base backup side as there is no actual point in
> > syncing segments while the backup is running and we could delay that
> > at the end of the backup (if I recall that stuff correctly).
>
> I don't have a good sample for these. One instance of this happening is
> below:
> 
> 0.000125 fsync(7)  = 0 <0.016677>
> 0.39 fsync(7)  = 0 <0.05>
> # 2048 writes for total of 16777216 bytes (16MB)
> 0.000618 write(7,
> "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
> 8192) = 8192 <0.21>
> 0.78 fsync(8)  = 0 <57.609720>
> 57.609830 fsync(8)  = 0 <0.07>
>
> Again, it is a problem with our network file system that we are still
> investigating. I'm not sure this can be improved easily, since
> pg_basebackup shares this code with walreceiver.
>

One workaround you could perhaps look at here is to run pg_basebackup
with --no-sync. That way there will be no fsyncs issued while running. You
will then of course have to take care of syncing all the files to disk
after it's done, but a network filesystem might be happier in dealing with
a large "batch-sync" like that rather than piece-by-piece sync.

(yes, I realize that wasn't your original question, just wanted to make
sure it was a workaround you had considered)

//Magnus


Re: WIP: Avoid creation of the free space map for small tables

2019-01-26 Thread Amit Kapila
On Sat, Jan 26, 2019 at 5:05 AM John Naylor  wrote:
>
> On Thu, Jan 24, 2019 at 5:19 AM Amit Kapila  wrote:
> >
> Performance testing is probably a good idea anyway, but I went ahead
> and implemented your next idea:
>
> > The other alternative is we can fetch pg_class.relpages and rely on
> > that to take this decision, but again if that is not updated, we might
> > take the wrong decision.
>
> We can think of it this way: Which is worse,
> 1. Transferring a FSM we don't need, or
> 2. Skipping a FSM we need
>
> I'd say #2 is worse.
>

Agreed.

> So, in v19 we check pg_class.relpages and if it's
> a heap and less than or equal the threshold we call stat on the 0th
> segment to verify.
>

Okay, but the way logic is implemented appears clumsy to me.

@@ -234,16 +243,40 @@ transfer_relfile(FileNameMap *map, const char
*type_suffix, bool vm_must_add_fro
  {
  /* File does not exist?  That's OK, just return */
  if (errno == ENOENT)
- return;
+ return first_seg_size;
  else
- pg_fatal("error while checking for file existence \"%s.%s\" (\"%s\"
to \"%s\"): %s\n",
- map->nspname, map->relname, old_file, new_file,
- strerror(errno));
+ goto fatal;
  }

  /* If file is empty, just return */
  if (statbuf.st_size == 0)
- return;
+ return first_seg_size;
+ }
+
+ /* Save size of the first segment of the main fork. */
+
+ else if (map->relpages <= HEAP_FSM_CREATION_THRESHOLD &&
+ (map->relkind == RELKIND_RELATION ||
+   map->relkind == RELKIND_TOASTVALUE))
+ {
+ /*
+ * In this case, if pg_class.relpages is wrong, it's possible
+ * that a FSM will be skipped when we actually need it.  To guard
+ * against this, we verify the size of the first segment.
+ */
+ if (stat(old_file, &statbuf) != 0)
+ goto fatal;
+ else
+ first_seg_size = statbuf.st_size;
+ }
+ else
+ {
+ /*
+ * For indexes etc., we don't care if pg_class.relpages is wrong,
+ * since we always transfer their FSMs.  For heaps, we might
+ * transfer a FSM when we don't need to, but this is harmless.
+ */
+ first_seg_size = Min(map->relpages, RELSEG_SIZE) * BLCKSZ;
  }

The function transfer_relfile has no clue about skipping of FSM stuff,
but it contains comments about it.  The check "if (map->relpages <=
HEAP_FSM_CREATION_THRESHOLD ..." will needlessly be executed for each
segment.  I think there is some value in using the information from
this function to skip fsm files, but the code doesn't appear to fit
well, how about moving this check to new function
new_cluster_needs_fsm()?


> In the common case, the cost of the stat call is
> offset by not linking the FSM.
>

Agreed.

> Despite needing another pg_class field,
> I think this code is actually easier to read than my earlier versions.
>

Yeah, the code appears cleaner from the last version, but I think we
can do more in that regards.

One more minor comment:
snprintf(query + strlen(query), sizeof(query) - strlen(query),
  "SELECT all_rels.*, n.nspname, c.relname, "
- "  c.relfilenode, c.reltablespace, %s "
+ "  c.relfilenode, c.reltablespace, c.relpages, c.relkind, %s "
  "FROM (SELECT * FROM regular_heap "
  "  UNION ALL "
  "  SELECT * FROM toast_heap "
@@ -525,6 +530,8 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
  i_relname = PQfnumber(res, "relname");
  i_relfilenode = PQfnumber(res, "relfilenode");
  i_reltablespace = PQfnumber(res, "reltablespace");
+ i_relpages = PQfnumber(res, "relpages");
+ i_relkind = PQfnumber(res, "relkind");
  i_spclocation = PQfnumber(res, "spclocation");

The order in which relkind and relpages is used in the above code is
different from the order in which it is mentioned in the query, it
won't matter, but keeping in order will make look code consistent.  I
have made this and some more minor code adjustments in the attached
patch.  If you like those, you can include them in the next version of
your patch.


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


v20-0003-During-pg_upgrade-conditionally-skip-transfer-of-FSM.patch
Description: Binary data


Re: Thread-unsafe coding in ecpg

2019-01-26 Thread Tom Lane
Michael Meskes  writes:
> I wonder if it would be better to make the test cases use the proper
> whenever command instead. That would give us a slightly better
> functionality testing I'd say.

I like having a hard limit on the number of loop iterations;
that should ensure that the test terminates no matter how confused
ecpglib is.

But certainly we could have more of the tests using "whenever"
as the intended method of getting out of the loop.

regards, tom lane



Re: [Patch] Log10 and hyperbolic functions for SQL:2016 compliance

2019-01-26 Thread Tom Lane
=?UTF-8?Q?L=C3=A6titia_Avrot?=  writes:
> [ adding_log10_and_hyperbolic_functions_v1.patch ]

No objection to the feature, but 

- Why are you using the float4-width library functions (coshf etc)
rather than the float8-width ones (cosh etc)?

- I wonder whether these library functions exist everywhere.
If they don't, what will we do about it ... throw an error?

I see that SUSv2 requires cosh() and so on, so it's quite possible
that these do exist everywhere we care about.  I'd be okay with
throwing a patch onto the buildfarm to see, and adding an autoconf
test only if the buildfarm is unhappy.  But we should be clear on
what we're going to do about it if that happens.

> I added regression tests for the new functions, but I didn't for log10
> function, assuming that if log function worked, log10 will work too.

Not quite sure I believe that.

Actually, what I'd counsel is that you *not* include tests of what
these do with Inf and NaN.  There is no upside to doing so, and lots
of downside if older platforms are squirrely in their behavior, which
is hardly unlikely (cf opossum ...)

regards, tom lane



Re: Use zero for nullness estimates of system attributes

2019-01-26 Thread Tom Lane
Jim Finnerty  writes:
> It's related, but what I was referring to applies even to the uncorrelated
> case: suppose you have something like:

> select x, sum(z) 
> from t
> where
> x > 5 and y in ('a', 'b', 'c')
> group by x;

> let's say that 'a', 'b', and 'c' are not frequent values of y, so the
> estimated selectivity is based on the n_distinct of y and the 3 values.  Now
> imagine that x > 5 is applied first.  That reduces the number of qualifying
> rows by the selectivity of (x > 5), but it may also reduce the number of
> distinct values of y.  If it reduces the n_distinct of y, then the IN
> predicate selectivity should be adjusted also.

I don't actually think that's a foregone conclusion.  If the two where
clauses are in fact independent, then simply multiplying their
selectivities together is the right thing.

regards, tom lane



Opossum vs. float4 NaN

2019-01-26 Thread Tom Lane
I see that opossum's owner just resurrected it after a period of being
MIA ... and it's failing a few regression tests with symptoms like
this:

 SELECT 'NaN'::float4;
- float4 
-
-NaN
+  float4  
+--
+ Infinity
 (1 row)

I have no doubt that this is caused by the same platform bug that
I worked around in commit cb3e9e40b:

However, it emerges that this tickles a compiler or hardware bug on
buildfarm member opossum.  Test results from commit 55e0e4581 show
conclusively that widening a float4 NaN to float8 produces Inf, not NaN,
on that machine; which accounts perfectly for the window RANGE test
failures it's been showing.  We can dodge this problem by making
in_range_float4_float8 be an independent function, so that it checks
for NaN inputs before widening them.

The reason that it's reappeared is the refactoring we've done recently
around snprintf: float4out is now taking its float4 argument and widening
it to double to pass to strtod, which is where the checks for Inf/NaN
happen.

We could imagine band-aiding around this by adding something like

#if defined(__netbsd) && defined(__mips)
if (isnan(num))
// return "NaN"
#endif

to float4out, but ick.  I was willing to do cb3e9e40b because it didn't
really make the code any uglier, but this would.  And I bet the issue
is going to cause problems somewhere for Andrew's Ryu patch, too.

I'm thinking we should regretfully retire opossum, unless there's
a software update available for it that fixes this bug.

regards, tom lane



Re: Opossum vs. float4 NaN

2019-01-26 Thread Andrew Gierth
> "Tom" == Tom Lane  writes:

 Tom> The reason that it's reappeared is the refactoring we've done
 Tom> recently around snprintf: float4out is now taking its float4
 Tom> argument and widening it to double to pass to strtod, which is
 Tom> where the checks for Inf/NaN happen.
 [...]
 Tom> to float4out, but ick. I was willing to do cb3e9e40b because it
 Tom> didn't really make the code any uglier, but this would. And I bet
 Tom> the issue is going to cause problems somewhere for Andrew's Ryu
 Tom> patch, too.

Actually I'd expect the reverse - Ryu never widens a float4 to float8,
it does everything in integers after extracting the bits from the value;
so it should be immune to this issue.

-- 
Andrew (irc:RhodiumToad)



Re: Allowing extensions to supply operator-/function-specific info

2019-01-26 Thread Tom Lane
I wrote:
> There's a considerable amount of follow-up work that ought to happen
> now to make use of these capabilities for places that have been
> pain points in the past, such as generate_series() and unnest().
> But I haven't touched that yet.

Attached is an 0004 that makes a stab at providing some intelligence
for unnest() and the integer cases of generate_series().  This only
affects one plan choice in the existing regression tests; I tweaked
that test to keep the plan the same.  I didn't add new test cases
demonstrating the functionality, since it's a bit hard to show it
directly within the constraints of EXPLAIN (COSTS OFF).  We could
do something along the lines of the quick-hack rowcount test in 0003,
perhaps, but that's pretty indirect.

Looking at this, I'm dissatisfied with the amount of new #include's
being dragged into datatype-specific .c files.  I don't really want
to end up with most of utils/adt/ having dependencies on planner
data structures, but that's where we would be headed.  I can think
of a couple of possibilities:

* Instead of putting support functions beside their target function,
group all the core's support functions into one new .c file.  I'm
afraid this would lead to the reverse problem of having to import
lots of datatype-private info into that file.

* Try to refactor the planner's .h files so that there's just one
"external use" header providing stuff like estimate_expression_value,
while keeping PlannerInfo as an opaque struct.  Then importing that
into utils/adt/ files would not represent such a big dependency
footprint.

I find the second choice more appealing, though it's getting a bit
far afield from where this started.  OTOH, lots of other header
refactoring is going on right now, so why not ...

Thoughts?

regards, tom lane

diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index e457d81..14cc202 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -22,12 +22,15 @@
 #include "catalog/pg_type.h"
 #include "funcapi.h"
 #include "libpq/pqformat.h"
+#include "nodes/supportnodes.h"
+#include "optimizer/clauses.h"
 #include "utils/array.h"
 #include "utils/arrayaccess.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
+#include "utils/selfuncs.h"
 #include "utils/typcache.h"
 
 
@@ -6026,6 +6029,36 @@ array_unnest(PG_FUNCTION_ARGS)
 	}
 }
 
+/*
+ * Planner support function for array_unnest(anyarray)
+ */
+Datum
+array_unnest_support(PG_FUNCTION_ARGS)
+{
+	Node	   *rawreq = (Node *) PG_GETARG_POINTER(0);
+	Node	   *ret = NULL;
+
+	if (IsA(rawreq, SupportRequestRows))
+	{
+		/* Try to estimate the number of rows returned */
+		SupportRequestRows *req = (SupportRequestRows *) rawreq;
+
+		if (is_funcclause(req->node))	/* be paranoid */
+		{
+			List	   *args = ((FuncExpr *) req->node)->args;
+			Node	   *arg1;
+
+			/* We can use estimated argument values here */
+			arg1 = estimate_expression_value(req->root, linitial(args));
+
+			req->rows = estimate_array_length(arg1);
+			ret = (Node *) req;
+		}
+	}
+
+	PG_RETURN_POINTER(ret);
+}
+
 
 /*
  * array_replace/array_remove support
diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index fd82a83..263920c 100644
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -30,11 +30,14 @@
 
 #include 
 #include 
+#include 
 
 #include "catalog/pg_type.h"
 #include "common/int.h"
 #include "funcapi.h"
 #include "libpq/pqformat.h"
+#include "nodes/supportnodes.h"
+#include "optimizer/clauses.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 
@@ -1427,3 +1430,73 @@ generate_series_step_int4(PG_FUNCTION_ARGS)
 		/* do when there is no more left */
 		SRF_RETURN_DONE(funcctx);
 }
+
+/*
+ * Planner support function for generate_series(int4, int4 [, int4])
+ */
+Datum
+generate_series_int4_support(PG_FUNCTION_ARGS)
+{
+	Node	   *rawreq = (Node *) PG_GETARG_POINTER(0);
+	Node	   *ret = NULL;
+
+	if (IsA(rawreq, SupportRequestRows))
+	{
+		/* Try to estimate the number of rows returned */
+		SupportRequestRows *req = (SupportRequestRows *) rawreq;
+
+		if (is_funcclause(req->node))	/* be paranoid */
+		{
+			List	   *args = ((FuncExpr *) req->node)->args;
+			Node	   *arg1,
+	   *arg2,
+	   *arg3;
+
+			/* We can use estimated argument values here */
+			arg1 = estimate_expression_value(req->root, linitial(args));
+			arg2 = estimate_expression_value(req->root, lsecond(args));
+			if (list_length(args) >= 3)
+arg3 = estimate_expression_value(req->root, lthird(args));
+			else
+arg3 = NULL;
+
+			/*
+			 * If any argument is constant NULL, we can safely assume that
+			 * zero rows are returned.  Otherwise, if they're all non-NULL
+			 * constants, we can calculate the number of rows that will be
+			 * returned.  Use double arithmetic to avoid overflow hazards.
+			 */
+			if ((IsA(arg1, Const) &&
+ ((Const *) arg

Re: Index Skip Scan

2019-01-26 Thread Dmitry Dolgov
> On Thu, Dec 20, 2018 at 2:46 PM Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> I've performed some testing, and on my environment with a dataset of 10^7
> records:
>
> * everything below 7.5 * 10^5 unique records out of 10^7 was faster with skip
>   scan.
>
> * above 7.5 * 10^5 unique records skip scan was slower, e.g. for 10^6 unique
>   records it was about 20% slower than the regular index scan.
>
> For me these numbers sound good, since even in quite extreme case of
> approximately 10 records per group the performance of index skip scan is close
> to the same for the regular index only scan.

Rebased version after rd_amroutine was renamed.


0001-Index-skip-scan-v6.patch
Description: Binary data


Re: Alternative to \copy in psql modelled after \g

2019-01-26 Thread Tom Lane
Fabien COELHO  writes:
>> It's unclear to me whether to push ahead with Daniel's existing
>> patch or not.  It doesn't look to me like it's making things
>> any worse from the error-consistency standpoint than they were
>> already, so I'd be inclined to consider error semantics cleanup
>> as something to be done separately/later.

> Fine.

OK.  I fixed the error-cleanup issue and pushed it.

The patch applied cleanly back to 9.5, but the code for \g is a good
bit different in 9.4.  I didn't have the interest to try to make the
patch work with that, so I just left 9.4 alone.

regards, tom lane



Re: Early WIP/PoC for inlining CTEs

2019-01-26 Thread Tom Lane
I wrote:
> Therefore, I'm reversing my previous opinion that we should not have
> an explicit NOT MATERIALIZED option.  I think we should add that, and
> the behavior ought to be:
> * No option given: inline if there's exactly one reference.
> * With MATERIALIZED: never inline.
> * With NOT MATERIALIZED: inline regardless of the number of references.
> (Obviously, we should not inline if there's RECURSIVE or the CTE
> potentially has side-effects, regardless of the user option;
> I don't think those cases are up for debate.)

Hearing no immediate pushback on that proposal, I went ahead and made
a version of the patch that does it like that, as attached.  I also took
a stab at documenting it fully.

I was interested to find, while writing the docs, that it's a real
struggle to invent plausible reasons to write MATERIALIZED given the
above specification.  You pretty much have to have lied to the planner,
eg by making a volatile function that's not marked volatile, before
there's a real need for that.  Am I missing something?  If I'm not,
then we're in a really good place backwards-compatibility-wise,
because the new default behavior shouldn't break any cases where people
weren't cheating.

regards, tom lane

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index f177eba..6d456f6 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2925,6 +2925,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
 
 /* we store the string name because RTE_CTE RTEs need it */
 APP_JUMB_STRING(cte->ctename);
+APP_JUMB(cte->ctematerialized);
 JumbleQuery(jstate, castNode(Query, cte->ctequery));
 			}
 			break;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index bb92d9d..8c26dd1 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1888,7 +1888,7 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
 
 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
  QUERY PLAN  
 -
  Limit
@@ -1905,7 +1905,7 @@ WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2
Output: t.c1_1, t.c2_1, t.c1_3
 (12 rows)
 
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
  c1_1 | c2_1 
 --+--
   101 |  101
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index f438165..56602a1 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -493,8 +493,8 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
 SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR SHARE;
 -- join in CTE
 EXPLAIN (VERBOSE, COSTS OFF)
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
-WITH t (c1_1, c1_3, c2_1) AS (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
+WITH t (c1_1, c1_3, c2_1) AS MATERIALIZED (SELECT t1.c1, t1.c3, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1)) SELECT c1_1, c2_1 FROM t ORDER BY c1_3, c1_1 OFFSET 100 LIMIT 10;
 -- ctid with whole-row reference
 EXPLAIN (VERBOSE, COSTS OFF)
 SELECT t1.ctid, t1, t2, t1.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10;
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index 88bc189..2225255 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -2199,16 +2199,19 @@ SELECT n FROM t LIMIT 100;
   
 
   
-   A useful property of WITH queries is that 

Re: Early WIP/PoC for inlining CTEs

2019-01-26 Thread Marko Tiikkaja
On Sat, Jan 26, 2019 at 12:22 AM Tom Lane  wrote:

> Therefore, I'm reversing my previous opinion that we should not have
> an explicit NOT MATERIALIZED option.  I think we should add that, and
> the behavior ought to be:
>
> * No option given: inline if there's exactly one reference.
>
> * With MATERIALIZED: never inline.
>
> * With NOT MATERIALIZED: inline regardless of the number of references.
>

This much has been obvious to most people for a long time.


.m


Race condition in crash-recovery tests

2019-01-26 Thread Tom Lane
Recently, buildfarm member curculio has started to show a semi-repeatable
failure in src/test/recovery/t/013_crash_restart.pl:

# aborting wait: program died
# stream contents: >>psql::8: no connection to the server
# psql::8: connection to server was lost
# <<
# pattern searched for: (?^m:server closed the connection unexpectedly)

#   Failed test 'psql query died successfully after SIGKILL'
#   at t/013_crash_restart.pl line 198.

The message this test is looking for is what libpq reports upon getting
EOF or ECONNRESET from a socket read attempt.  The message it's actually
seeing is what libpq reports if it notices that the PQconn is *already*
in CONNECTION_BAD state when it's trying to send a new query.

I have no idea why we're seeing this in only one buildfarm member
and only for the past week or so, as it doesn't appear that any
related code has changed for months.  (Perhaps something changed
about curculio's host?)

I do see a plausibly related change back in October: commit 4247db625
added PQconsumeInput() calls in places where psql would call that after
having already emitted a query result, but before going idle.  So my
rough theory about what's happening is that psql loses the CPU immediately
after printing the query result containing "in-progress-before-sigquit",
which allows the test script to proceed, and doesn't get it back until
after the connected backend commits hara-kiri.  Then PQconsumeInput
tries to read the socket, gets EOF or ECONNRESET, and sets a message
(that's ignored) along with setting state = CONNECTION_BAD.  Then when
psql tries to send the next query, we get the observed message instead
of what the test is expecting.

I'm inclined to write this off as an inevitable race condition and
just change the test script to accept either message as a successful
result.  I think that 4247db625 made such races more likely, but I
don't believe it was impossible before.

Another idea is to change libpq so that both these cases emit identical
messages, but I don't really feel that that'd be an improvement.  Also,
since 4247db625 was back-patched, we'd have to back-patch the message
change as well, which I like even less.  People might be relying on
seeing either message spelling in some situations.

In any case, it'd be nice to have an explanation of why this just
started failing --- that fact makes it even harder to credit that
4247db625 was the trigger.

Thoughts?

regards, tom lane



Re: Race condition in crash-recovery tests

2019-01-26 Thread Andres Freund
Hi,

On 2019-01-26 20:53:48 -0500, Tom Lane wrote:
> Recently, buildfarm member curculio has started to show a semi-repeatable
> failure in src/test/recovery/t/013_crash_restart.pl:
> 
> # aborting wait: program died
> # stream contents: >>psql::8: no connection to the server
> # psql::8: connection to server was lost
> # <<
> # pattern searched for: (?^m:server closed the connection unexpectedly)
> 
> #   Failed test 'psql query died successfully after SIGKILL'
> #   at t/013_crash_restart.pl line 198.
> 
> The message this test is looking for is what libpq reports upon getting
> EOF or ECONNRESET from a socket read attempt.  The message it's actually
> seeing is what libpq reports if it notices that the PQconn is *already*
> in CONNECTION_BAD state when it's trying to send a new query.
> 
> I have no idea why we're seeing this in only one buildfarm member
> and only for the past week or so, as it doesn't appear that any
> related code has changed for months.  (Perhaps something changed
> about curculio's host?)

I have no idea why it's just curculio, but I think I know why it only
started recently: Curculio doesn't appear to have tap tests enabled
before
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=curculio&dt=2019-01-17%2021%3A30%3A02


> just change the test script to accept either message as a successful
> result.  I think that 4247db625 made such races more likely, but I
> don't believe it was impossible before.

Sounds right to me - do you want to do the honors or shall I?


> Another idea is to change libpq so that both these cases emit identical
> messages, but I don't really feel that that'd be an improvement.  Also,
> since 4247db625 was back-patched, we'd have to back-patch the message
> change as well, which I like even less.  People might be relying on
> seeing either message spelling in some situations.

Yea, I don't think that's the way to go.

Greetings,

Andres Freund



Re: Variable-length FunctionCallInfoData

2019-01-26 Thread Andres Freund
Hi,

On 2019-01-25 12:51:02 -0800, Andres Freund wrote:
> Updated patch attached.  Besides the above changes, there's a fair bit
> of comment changes, and I've implemented the necessary JIT changes.

I pushed a further polished version of this.

The buildfarm largely seems to have had no problem with it, but handfish
failed:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=handfish&dt=2019-01-26%2022%3A57%3A19
but I have no idea what the error is, nor whether it's related to this
failure, because for reasons I do not understand, the stage message is
effectively empty.   Going to wait for another run before investing
resources to debug...

Regards,

Andres



Re: Race condition in crash-recovery tests

2019-01-26 Thread Tom Lane
Andres Freund  writes:
> On 2019-01-26 20:53:48 -0500, Tom Lane wrote:
>> I have no idea why we're seeing this in only one buildfarm member
>> and only for the past week or so, as it doesn't appear that any
>> related code has changed for months.  (Perhaps something changed
>> about curculio's host?)

> I have no idea why it's just curculio, but I think I know why it only
> started recently: Curculio doesn't appear to have tap tests enabled
> before
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=curculio&dt=2019-01-17%2021%3A30%3A02

Oh, right ... I knew that, actually, but forgot ...

So then we only have to assume that the race condition is encouraged
by something about the kernel scheduler's rules on that machine, which
isn't so much of a leap, especially since it's our only OpenBSD
critter.  The test case only exists in v11 and HEAD branches, and
curculio's only run this test a few times in v11, so the lack of
back-branch failures isn't so odd.

>> just change the test script to accept either message as a successful
>> result.  I think that 4247db625 made such races more likely, but I
>> don't believe it was impossible before.

> Sounds right to me - do you want to do the honors or shall I?

I'll do it in a bit.

regards, tom lane



Re: Variable-length FunctionCallInfoData

2019-01-26 Thread Tom Lane
Andres Freund  writes:
> The buildfarm largely seems to have had no problem with it, but handfish
> failed:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=handfish&dt=2019-01-26%2022%3A57%3A19
> but I have no idea what the error is, nor whether it's related to this
> failure, because for reasons I do not understand, the stage message is
> effectively empty.

Yeah, we've been seeing random failures with empty stage logs on multiple
machines.  I suspect something weird with the buildfarm client script,
but no idea what.

regards, tom lane



relcache reference leak with pglogical replication to insert-only partitioned table?

2019-01-26 Thread Jeremy Finzel
I understand it's not fully supported to replicate to a differently
partitioned setup on a subscriber with either pglogical or the native
logical replication, however I also know that INSERT triggers can be fired
in replication mode.  I have an insert-only OLTP table that I want
partitioned only on the subscriber system.  I have this setup using the
"old style" partitioning as it is a 9.6 system.

Provider is 9.6.6 pglogical 2.1.1
Subscriber is 9.6.10 pglogical 2.1.1

Everything appears good as far as the data.  It is partitioning correctly.
Queries on the data are planning correctly.  However, I am now getting
these WARNING messages constantly.  How concerned should I be?  Is there a
fix for this?  Any insight is much appreciated!

2019-01-27 03:12:34.150 GMT,,,135600,,5c4d1f44.211b0,6794,,2019-01-27
03:02:28 GMT,54/0,1057372660,WARNING,01000,"relcache reference leak:
relation ""foo_pkey"" not closed","apply COMMIT in commit before
14DB/34DB1B78, xid 1476598649 commited at 2019-01-26 21:12:34.071673-06
(action #10) from node replorigin 22""pglogical apply 16420:2094659706"

Thank you!
Jeremy


Re: Variable-length FunctionCallInfoData

2019-01-26 Thread Andrew Dunstan


On 1/26/19 9:46 PM, Tom Lane wrote:
> Andres Freund  writes:
>> The buildfarm largely seems to have had no problem with it, but handfish
>> failed:
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=handfish&dt=2019-01-26%2022%3A57%3A19
>> but I have no idea what the error is, nor whether it's related to this
>> failure, because for reasons I do not understand, the stage message is
>> effectively empty.
> Yeah, we've been seeing random failures with empty stage logs on multiple
> machines.  I suspect something weird with the buildfarm client script,
> but no idea what.


It's happening on a handful of animals (see below) , and at that
intermittently. Some of the animals are leading or bleeding edge
distros. Without more info I'm inclined to suspect environmental factors.


cheers


andrew


pgbfprod=> select sysname, branch, snapshot, log_stage from
public.build_status_log where snapshot > now() - interval '90 days' and
length(log_text) = 0 order by 1,3,2;
  sysname   |    branch |  snapshot   |   
log_stage   
+---+-+-
 caiman | REL_11_STABLE | 2018-10-30 05:00:06 | configure.log
 caiman | REL_10_STABLE | 2018-11-05 01:00:18 | check-pg_upgrade.log
 caiman | REL_11_STABLE | 2018-11-05 01:05:16 | make.log
 caiman | REL9_4_STABLE | 2018-11-06 02:57:28 | make.log
 caiman | REL9_5_STABLE | 2018-11-06 03:00:08 | configure.log
 caiman | REL_10_STABLE | 2018-11-06 03:12:50 | configure.log
 caiman | HEAD  | 2018-11-06 03:25:58 | make.log
 caiman | HEAD  | 2018-11-06 21:55:13 | make.log
 caiman | HEAD  | 2018-11-09 06:15:27 | make.log
 caiman | HEAD  | 2018-11-14 08:53:13 |
contrib-install-check-C.log
 caiman | HEAD  | 2018-11-17 19:58:06 | make-install.log
 caiman | REL9_6_STABLE | 2018-11-26 02:00:23 | make.log
 caiman | REL_11_STABLE | 2018-11-26 02:12:53 | make.log
 caiman | HEAD  | 2018-12-05 13:11:32 | configure.log
 caiman | REL_11_STABLE | 2018-12-16 20:48:41 | make.log
 caiman | REL9_5_STABLE | 2018-12-17 01:58:40 | check.log
 caiman | REL9_6_STABLE | 2018-12-17 02:00:07 | make.log
 caiman | REL_11_STABLE | 2018-12-17 02:10:13 | make.log
 caiman | REL_10_STABLE | 2018-12-18 18:00:14 | configure.log
 caiman | HEAD  | 2018-12-20 22:00:31 | make-contrib.log
 caiman | REL_11_STABLE | 2018-12-24 11:54:44 | lastcomand.log
 caiman | HEAD  | 2018-12-24 12:00:13 | configure.log
 caiman | HEAD  | 2019-01-02 19:00:21 | configure.log
 caiman | REL_11_STABLE | 2019-01-20 00:54:29 | make.log
 caiman | REL_11_STABLE | 2019-01-22 06:00:25 | configure.log
 caiman | REL9_6_STABLE | 2019-01-26 03:50:26 | make.log
 caiman | HEAD  | 2019-01-26 04:14:53 | make.log
 eelpout    | HEAD  | 2018-11-26 00:35:13 | make.log
 eelpout    | HEAD  | 2018-11-26 00:35:13 | configure.log
 eelpout    | HEAD  | 2018-11-26 00:35:13 | make-contrib.log
 handfish   | REL_11_STABLE | 2018-10-31 00:46:56 | make.log
 handfish   | HEAD  | 2018-10-31 01:00:33 | configure.log
 handfish   | REL_11_STABLE | 2018-11-02 22:25:25 | configure.log
 handfish   | REL_11_STABLE | 2018-11-05 01:05:18 | make.log
 handfish   | REL_11_STABLE | 2018-11-06 03:58:58 | make.log
 handfish   | HEAD  | 2018-11-06 21:51:39 | make.log
 handfish   | REL_11_STABLE | 2018-11-07 01:03:16 | make.log
 handfish   | REL9_5_STABLE | 2018-11-09 05:59:24 | make.log
 handfish   | REL_11_STABLE | 2018-11-09 06:24:31 | make.log
 handfish   | REL_11_STABLE | 2018-11-12 01:00:15 | make.log
 handfish   | HEAD  | 2018-11-26 02:13:04 | make.log
 handfish   | REL_11_STABLE | 2018-12-05 13:11:12 | configure.log
 handfish   | REL_11_STABLE | 2018-12-16 20:45:08 | make.log
 handfish   | REL9_4_STABLE | 2018-12-17 01:55:31 | initdb-C.log
 handfish   | REL_10_STABLE | 2018-12-17 02:09:59 | make.log
 handfish   | HEAD  | 2018-12-18 18:25:55 | make.log
 handfish   | HEAD  | 2018-12-24 12:04:56 | make.log
 handfish   | REL9_6_STABLE | 2019-01-01 16:57:14 | ecpg-check.log
 handfish   | REL_10_STABLE | 2019-01-01 17:05:17 | configure.log
 handfish   | HEAD  | 2019-01-26 05:11:49 | make.log
 handfish   | HEAD  | 2019-01-26 22:57:19 |
testmodules-install-check-C.log
 queensnake | REL9_6_STABLE | 2018-11-26 01:57:27 | check.log
 queensnake | REL_10_STABLE | 2018-11-26 02:00:14 | configure.log
 queensnake | REL_11_STABLE | 2018-11-30 04:57:15 | check-pg_upgrade.log
 queensnake | REL_10_STABLE | 2018-12-05 12:57:28 | make-install.log
 queensnake | REL_11_STABLE | 2018-12-16 20:44:19 | make-contrib.log
 queensnake | REL9_5_STABLE | 2018-12-17 01:57:36 | check-pg_upgrade.log
 queensnake | HEAD  | 2018-12-20 22:09:20 | configure.log
 queensnake | HEAD  | 2019-01-20 00:23:49 | make-contrib.log
 queensnake | HEAD  | 2

Re: Variable-length FunctionCallInfoData

2019-01-26 Thread Greg Stark
I assume you already considered and rejected having a fixed size null
bitmap followed by a variable size array of datums. That seems like it
would be denser and work better with cpu cache.

I guess the reason you prefer the struct is because it can be used
elsewhere on its own?


Re: Variable-length FunctionCallInfoData

2019-01-26 Thread Andres Freund
Hi,

On 2019-01-27 08:03:17 +0100, Greg Stark wrote:
> I assume you already considered and rejected having a fixed size null
> bitmap followed by a variable size array of datums. That seems like it
> would be denser and work better with cpu cache.

It'd be more expensive to access individually (offset calculation +
masks, ~5 insn, not fully pipelineable), it'd not guarantee that the
null bit and datum are on the same cacheline, you could not pass the
null-bit to various functions accepting a bool*, you could not pass the
new equivalent NullableDatums to other functions (like both the past and
current solution allow).

Greetings,

Andres Freund



Re: pgsql: Remove references to Majordomo

2019-01-26 Thread Noah Misch
On Sat, Jan 19, 2019 at 01:19:46PM -0500, Stephen Frost wrote:
> * Magnus Hagander (mag...@hagander.net) wrote:
> > On Fri, Jan 18, 2019 at 4:02 PM Tom Lane  wrote:
> > > Magnus Hagander  writes:
> > > > On Fri, Jan 18, 2019 at 1:26 AM Michael Paquier 
> > > wrote:
> > > >> Wouldn't it be better to also switch the references to pgsql-bugs in
> > > >> all the C code for the different --help outputs?
> > >
> > > > You are right, we definitely should. I'll go ahead and fix that. I can't
> > > > quite make up my mind on if it's a good idea to backpatch that though --
> > > > it's certainly safe enough to do, but it might cause issues for
> > > translators?
> > >
> > > Yeah, weak -1 for back-patching.  We don't usually like to thrash
> > > translatable messages in the back branches.
> > 
> > Pushed.
> 
> Does this also implicitly mean we've just agreed to push back the
> retirement of the @postgresql.org aliases for the lists until v11 is
> EOL..?
> 
> I can understand the concern around translators and back-patching and
> such, but I don't think we should be waiting another 5 years before we
> retire those aliases as having them is preventing us from moving forward
> with other infrastructure improvements to our email systems.

What are those blocked infrastructure improvements?