[HACKERS] Race condition when starting and stopping test server in TestLib.pm?

[Michael Paquier]

Hi all,

Within the last couple of days hamster has failed twice when running
TAP tests because it was not able to start a test server with
start_test_server:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamster&dt=2015-04-30%2019%3A07%3A31
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamster&dt=2015-04-22%2016%3A00%3A33

And here is the failure:
pg_ctl: could not start server
Examine the log output.
Bailout called.  Further testing stopped:  pg_ctl failed
cannot remove directory for
/home/buildfarm/data/buildroot/HEAD/pgsql.build/src/bin/scripts/tmp_testrTRB/pgdata:
Directory not empty at /usr/share/perl5/core_perl/File/Temp.pm line
784.
cannot remove directory for
/home/buildfarm/data/buildroot/HEAD/pgsql.build/src/bin/scripts/tmp_testrTRB:
Directory not empty at /usr/share/perl5/core_perl/File/Temp.pm line
784.

When connecting to the server this afternoon I noticed that indeed
that an instance of Postgres was still running, and visibly it has
been been able to stop because the temporary data directory has been
removed before the server stop really completed. Note that I stopped
it brutally for now to allow the next rounds of tests to work btw...

hamster is now known to be legendary slow in the buildfarm, so I guess
that it is good in catching up such race conditions, hence I am
wondering if the failure is not caused by one even if switch -w is
used when stopping the server in TestLib.pm.
END
{
if ($test_server_datadir)
{
system 'pg_ctl', '-D', $test_server_datadir, '-s', '-w', '-m',
  'immediate', 'stop';
}
}
Thoughts?
-- 
Michael


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

Re: [HACKERS] alter user/role CURRENT_USER

[Kyotaro HORIGUCHI]

Hello,

At Thu, 30 Apr 2015 17:12:25 -0300, Alvaro Herrera  
wrote in <20150430201225.gv4...@alvh.no-ip.org>
> Kyotaro HORIGUCHI wrote:
> > Thank you for completing this and very sorry not to respond these
> > days.
> > 
> > I understood that it is committed after I noticed that rebasing
> > my code failed..
> 
> You'd do well to check your email, I guess :-)

Yeah, I agree with you since I noticed that before I read the
mail mentioning that. I should be more carefull:( Sorry to bother
you and thank you for your kindness.

> > | =# alter role current_user rename to "PubLic";
> > | ERROR:  CURRENT_USER cannot be used as a role name
> > | LINE 1: alter role current_user rename to "PubLic";
> > |^
> > 
> > The error message sounds somewhat different from the intention. I
> > think the following message would be clearer.
> > 
> > | ERROR:  CURRENT_USER cannot be used as a role name here
> 
> Okay, changed.
> 
> 
> > 
> > The document sql-altergroup.html says
> > 
> > | ALTER GROUP role_specification ADD USER user_name [, ... ]
> > 
> > But current_user is also usable in user_name list. So the doc
> > should be as following, but it would not be necessary to be fixed
> > because it is an obsolete commnand..
> > 
> > | ALTER GROUP role_specification ADD USER role_specification [, ... ]
> 
> Yeah, EDONTCARE.
> 
> > "ALTER GROUP role_spec ADD/DROP USER role_spec" is naturally
> > denied so I think no additional description is needed.
> 
> +1
> 
> > 
> > sql-alterpolicy.html
> > 
> > "ALTER POLICY name ON table_name TO" also accepts current_user
> > and so as the role to which the policy applies.
> 
> Changed.
> 
> > # As a different topic, the syntax "ALTER POLICY  ON
> > #  TO " looks a bit wired, it might be better be to
> > # be "ON  APPLY TO " but I shouldn't try to fix it
> > # since it is a long standing syntax..
> 
> Yeah, it's a bit strange.  Not a strong opinion.  Maybe you should raise
> it as a separate thread.
> 
> > 
> > sql-createtablespace.html
> > sql-drop-owned.html, sql-reassign-owned.html
> 
> Changed.

Thank you applying the changes above.

> > ==
> > sql-grant.html, sql-revoke.html,
> > 
> > "GRANT  TO " and "REVOKE  FROM " are
> > the modern equivalents of the deprecated syntaxes "ALTER 
> > ADD USER " and "ALTER  DROP USER "
> > respectively. But the current parser infrastructure doesn't allow
> > coexistence of the two following syntaxes but I couldn't find the
> > way to their coexistence.
> 
> I decided to leave this out.  I think we should consider it as a new
> patch for 9.6; these changes aren't as clear-cut as the rest of your
> patch.  I didn't want to have to research the ecpg changes.

Ok, it sounds fair enough.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] Make more portable TAP tests of initdb

[Michael Paquier]

On Thu, Apr 30, 2015 at 12:17 PM, Noah Misch  wrote:
> On Wed, Apr 15, 2015 at 02:59:55PM +0900, Michael Paquier wrote:
>> On Wed, Apr 15, 2015 at 2:38 PM, Noah Misch wrote:
>> > Solaris 10 ships Perl 5.8.4, and RHEL 5.11 ships Perl 5.8.8.  Therefore, 
>> > Perl
>> > installations lacking this File::Path feature will receive vendor support 
>> > for
>> > years to come.  Replacing the use of keep_root with rmtree+mkdir will add 
>> > 2-10
>> > lines of code, right?  Let's do that and not raise the system requirements.
>>
>> Good point. Let's use mkdir in combination then. Attached is an updated 
>> patch.
>
>> --- a/src/bin/initdb/t/001_initdb.pl
>> +++ b/src/bin/initdb/t/001_initdb.pl
>> @@ -1,6 +1,7 @@
>>  use strict;
>>  use warnings;
>>  use TestLib;
>> +use File::Path qw(rmtree);
>>  use Test::More tests => 19;
>>
>>  my $tempdir = TestLib::tempdir;
>> @@ -18,27 +19,30 @@ command_fails([ 'initdb', '-S', "$tempdir/data3" ],
>>  mkdir "$tempdir/data4" or BAIL_OUT($!);
>>  command_ok([ 'initdb', "$tempdir/data4" ], 'existing empty data directory');
>>
>> -system_or_bail "rm -rf '$tempdir'/*";
>> -
>> +rmtree($tempdir);
>> +mkdir $tempdir;
>
> It's usually wrong to remove and recreate the very directory made by
> File::Temp.  Doing so permits a race condition: an attacker can replace the
> directory between the rmdir() and the mkdir().  However, TestLib::tempdir
> returns a subdirectory of the build directory, and the build directory is
> presumed secure.  That's good enough.

OK, I haven't thought of that.

>> -system_or_bail "rm -rf '$tempdir'/*";
>> +rmtree($tempdir);
>>  command_ok([ 'initdb', '-T', 'german', "$tempdir/data" ],
>>   'select default dictionary');
>
> You omitted the mkdir() on that last one.  It works, since initdb does the
> equivalent of "mkdir -p", but it looks like an oversight.
>
>
> As I pondered this, I felt it would do better to solve a different problem.
> The "rm -rf" invocations presumably crept in to reduce peak disk usage.
> Considering the relatively-high duration of a successful initdb run, I doubt
> we get good value from so many positive tests.  I squashed those positive
> tests into one, as attached.  (I changed the order of negative tests but kept
> them all.)  This removed the need for intra-script cleaning, and it
> accelerated script runtime from 22s to 9s.  Does this seem good to you?

That's a fight code simplicity vs. test performance. FWIW, I like the
test suite with many positive tests because it is easy to read the
test flow. And as this test suite is meant to grow up with new tests
in the future, I am guessing that people may not follow the
restriction this patch adds all the time.

 command_fails(
-   [ 'initdb', "$tempdir/data", '-X', 'pgxlog' ],
+   [ 'initdb', $datadir, '-X', 'pgxlog' ],
'relative xlog directory not allowed');
$datadir should be the last argument. This would break on platforms
where src/port/getopt_long.c is used, like Windows.

+command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ],
+   'successful creation');
This is grouping the test for nosync, existing nonempty xlog
directory, and the one for selection of the default dictionary.
Shouldn't this test name be updated otherwise then?

Could you add a comment mentioning that tests are grouped to reduce
I/O impact during a run?
Regards,
-- 
Michael


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

Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0

[Peter Geoghegan]

On Thu, Apr 30, 2015 at 7:00 PM, Heikki Linnakangas  wrote:
> To fix that, we need to fix the "livelock insurance" check so that A does
> not wait for B here. Because B is not a speculative insertion, A should
> cancel its speculative insertion and retry instead. (I pushed the one-line
> fix for that to your github repository)

I've been unable to reproduce the unprincipled deadlock using the same
test case as before. However, the exclusion constraint code now
livelocks. Here is example output from a stress-testing session:

trying 128 clients:
[Fri May  1 04:45:15 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 04:45:15 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 04:45:19 2015] sum is 96
[Fri May  1 04:45:19 2015] count is 8906
[Fri May  1 04:45:19 2015] normal exit at 1430455519 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 04:45:19 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 04:45:19 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 04:45:23 2015] sum is -610
[Fri May  1 04:45:23 2015] count is 8867
[Fri May  1 04:45:23 2015] normal exit at 1430455523 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 04:45:23 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 04:45:23 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 04:45:27 2015] sum is 352
[Fri May  1 04:45:27 2015] count is 8861
[Fri May  1 04:45:27 2015] normal exit at 1430455527 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 04:45:27 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 04:45:27 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 04:45:31 2015] sum is 190
[Fri May  1 04:45:31 2015] count is 8895
[Fri May  1 04:45:31 2015] normal exit at 1430455531 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 04:45:31 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 04:45:31 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 04:45:35 2015] sum is -76
[Fri May  1 04:45:35 2015] count is 8833
[Fri May  1 04:45:35 2015] normal exit at 1430455535 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 04:45:58 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 04:45:58 2015] init done at count_upsert_exclusion.pl line 106.


(I ssh into server, check progress). Then, due to some issue with the
scheduler or something, progress continues:

[Fri May  1 05:17:57 2015] sum is 462
[Fri May  1 05:17:57 2015] count is 8904
[Fri May  1 05:17:58 2015] normal exit at 1430457478 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 05:17:58 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 05:17:58 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 05:18:01 2015] sum is 316
[Fri May  1 05:18:01 2015] count is 8839
[Fri May  1 05:18:01 2015] normal exit at 1430457481 after 128000
items processed at count_upsert_exclusion.pl line 192.

Note that it's much easier to see non-uniform durations for each run
for no good reason, rather than livelock per say. Most runs are 3-4
seconds, but then every so often one will last 25 seconds for no good
reason. So maybe this is better described as a lock starvation issue:

trying 128 clients:
[Fri May  1 05:41:16 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 05:41:16 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 05:41:19 2015] sum is -264
[Fri May  1 05:41:19 2015] count is 8886
[Fri May  1 05:41:20 2015] normal exit at 1430458880 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 05:41:20 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 05:41:20 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 05:41:43 2015] sum is -14
[Fri May  1 05:41:43 2015] count is 8894
[Fri May  1 05:41:43 2015] normal exit at 1430458903 after 128000
items processed at count_upsert_exclusion.pl line 192.
trying 128 clients:
[Fri May  1 05:41:43 2015] NOTICE:  extension "btree_gist" already
exists, skipping
[Fri May  1 05:41:43 2015] init done at count_upsert_exclusion.pl line 106.
[Fri May  1 05:41:47 2015] sum is -338
[Fri May  1 05:41:47 2015] count is 8867
[Fri May  1 05:41:47 2015] normal exit at 1430458907 after 128000
items processed at count_upsert_exclusion.pl line 192.

If I look at the Postgres server log itself, I see very long running
queries that match the following pattern:

2015-05-01 06:06:42 UTC [ 0 ]: LOG:  duration: 21855.521 ms
statement: delete from upsert_race_test where index='9399' and count=0
2015-05-01 06:06:42 UTC [ 

Re: [HACKERS] feature freeze and beta schedule

[Michael Paquier]

On Fri, May 1, 2015 at 1:55 AM, Robert Haas  wrote:
> On Thu, Apr 30, 2015 at 12:52 PM, Peter Eisentraut  wrote:
>> On 4/30/15 12:01 PM, Robert Haas wrote:
>>> So generally we have stamped in late April or early May and released
>>> in September, but last year we didn't release until December.  I
>>> assume that if we stamp beta1 in June instead of May, that's going to
>>> somewhat delay the final release as well, but I'm not sure how much.
>>
>> The idea when that schedule was cobbled together in the dying minutes of
>> the last developer meeting ;-) was to have a shorter beta and still
>> shoot for a September release.
>
> I think that could be doable if we can keep focus, but that's easier
> said than done.

Just to make people aware here that I am not slacking: I am going to
be out of town for a couple of weeks more or less until the end of May
with limited access to computer so I am not sure I will be able to
help much.
Regards,
-- 
Michael


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

Re: [HACKERS] Faster setup_param_list() in plpgsql

[Pavel Stehule]

Review:

What this patch does - it change a mechanism, how a values of variables are
transmitted to SPI. In previous variant values are copied to ParamListInfo
before every evaluation of any expression. New mechanism is smarter. It
refresh only ROW, REC values when are marked as dirty (when these variables
was used). ParamListInfo is dynamically updated when value is assigned to
variable.

This patch can significantly reduce a overhead related to preparing
parameters - more it cleaning code.

1. There are no problem with patching, regress tests
2. The changes are clean and well documented
3. I can confirm 10% of speedup.

This patch is ready for commit.

Regards

Pavel Stehule

2015-04-30 20:50 GMT+02:00 Pavel Stehule :

>
>
> 2015-04-29 9:26 GMT+02:00 Pavel Stehule :
>
>> Hi all
>>
>> I am looking on this patch. I can confirm 10-15% speedup - and the idea
>> behind this patch looks well.
>>
>> This patch
>> http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us
>> contains two parts
>>
>> a) relative large refactoring
>>
>> b) support for resettable fields in param_list instead total reset
>>
>> I believe it should be in two separate patches. Refactoring is trivial
>> and there is no any possible objection.
>>
>
> I was wrong, there is relative strong dependency between these two parts,
> so it should be commited as one patch
>
> Regards
>
> Pavel
>
>
>>
>> Regards
>>
>> Pavel
>>
>
>

Re: [HACKERS] pg_rewind test race condition..?

[Heikki Linnakangas]

On 04/29/2015 06:03 AM, Stephen Frost wrote:

* Heikki Linnakangas (hlinn...@iki.fi) wrote:

--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7173,7 +7173,10 @@ StartupXLOG(void)
 * than is appropriate now that we're not in standby mode anymore.
 */
if (fast_promoted)
+   {
+   sleep(5);
RequestCheckpoint(CHECKPOINT_FORCE);
+   }
  }

The simplest fix would be to force a checkpoint in the regression
test, before running pg_rewind. It's a bit of a cop out, since you'd
still get the same issue when you tried to do the same thing in the
real world. It should be rare in practice - you'd not normally run
pg_rewind immediately after promoting the standby - but a better
error message at least would be nice..


Forcing a checkpoint in the regression tests and then providing a better
error message sounds reasonable to me.  I agree that it's very unlikely
to happen in the real world, even when you're bouncing between systems
for upgrades, etc, you're unlikely to do it fast enough for this issue
to exhibit itself, and a better error message would help any users who
manage to run into this (perhaps during their own testing).


I've committed this simple fix for now.
- 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] BuildTupleFromCStrings Memory Documentation?

[Tom Lane]

Jason Petersen  writes:
> Within the core codebase, BuildTupleFromCStrings is often called within a 
> temporary memory context cleared after the call. In dblink.c, this is 
> justified as being needed to “[clean up] not only the data we have direct 
> access to, but any cruft the I/O functions might leak”.
> I wrote a pretty minimal case to call BuildTupleFromCStrings in a loop 
> (attached) and found that I was using 40GB of RAM in a few minutes, though I 
> was not allocating any memory myself and immediately freed the tuple it 
> returned.

> Is the need to wrap this call in a protective context documented anywhere? 
> Portions of the documentation use BuildTupleFromCStrings in examples without 
> mentioning this precaution. Is it just well-known, or did I miss a README or 
> comment somewhere?

Most uses of BuildTupleFromCStrings only do it once per invocation of the
calling function, so that an outer-level reset of the calling function's
evaluation context is what takes care of any memory leaked by the I/O
functions BuildTupleFromCStrings invokes.  If you intend to call it many
times within the same function call then you should use a context you can
reset between calls.  This risk is hardly unique to BuildTupleFromCStrings,
which is why the documentation doesn't make a big point of it.

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] transforms vs. CLOBBER_CACHE_ALWAYS

[Peter Eisentraut]

On 4/30/15 2:49 PM, Andrew Dunstan wrote:
> friarbird is a FreeBSD buildfarm animal running with
> -DCLOBBER_CACHE_ALWAYS. It usually completes a run in about 6.5 hours.
> However, it's been stuck since Monday running the plpython regression
> tests. The only relevant commit seems to be the transforms feature.

I can reproduce it.  I'll look into it.


-- 
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] shared_libperl, shared_libpython

[Peter Eisentraut]

On 4/28/15 11:48 PM, Tom Lane wrote:
>> My preference would be to rip all that out and let the compiler or
>> linker decide when it doesn't want to link something.
> 
> Works for me, assuming that we get an understandable failure message and
> not, say, a plperl.so that mysteriously doesn't work.

Well, I can't really guarantee that, and I also recall that in some
cases a shared/non-shared mismatch will work but be slow or something
like that.

So I went for the configure detection.  For Perl, this is
straightforward.  For Python and Tcl, it gets tricky because they look
at the actual file in some cases, which requires knowing DLSUFFIX, which
is not available in configure.  (And it's a lie anyway, because on OS X
it does not distinguish between .so and .dylib in a principled way.)
For Tcl, this is only necessary for version before Tcl 8.0 (according to
code comments), which I think we can safely drop.  For Python, it's
still necessary, so I hardcoded a few choices in an ugly way.

I think overall this patch is still an improvement in several ways.
Worst case, we could turn some of these configure errors into warnings.
diff --git a/config/python.m4 b/config/python.m4
index 7012c53..c8f784e 100644
--- a/config/python.m4
+++ b/config/python.m4
@@ -93,7 +93,6 @@ AC_MSG_RESULT([${python_libspec} ${python_additional_libs}])
 AC_SUBST(python_libdir)[]dnl
 AC_SUBST(python_libspec)[]dnl
 AC_SUBST(python_additional_libs)[]dnl
-AC_SUBST(python_enable_shared)[]dnl
 
 # threaded python is not supported on OpenBSD
 AC_MSG_CHECKING(whether Python is compiled with thread support)
diff --git a/configure b/configure
index 7c0bd0c..cddbbef 100755
--- a/configure
+++ b/configure
@@ -641,7 +641,6 @@ TCL_SHLIB_LD_LIBS
 TCL_SHARED_BUILD
 TCL_LIB_SPEC
 TCL_LIBS
-TCL_LIB_FILE
 TCL_INCLUDE_SPEC
 TCL_CONFIG_SH
 TCLSH
@@ -662,7 +661,6 @@ HAVE_IPV6
 LIBOBJS
 UUID_LIBS
 ZIC
-python_enable_shared
 python_additional_libs
 python_libspec
 python_libdir
@@ -7384,6 +7382,12 @@ perl_useshrplib=`$PERL -MConfig -e 'print $Config{useshrplib}'`
 test "$PORTNAME" = "win32" && perl_useshrplib=`echo $perl_useshrplib | sed 's,,/,g'`
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $perl_useshrplib" >&5
 $as_echo "$perl_useshrplib" >&6; }
+  if test "$perl_useshrplib" != yes && test "$perl_useshrplib" != true; then
+as_fn_error $? "cannot build PL/Perl because libperl is not a shared library
+You might have to rebuild your Perl installation.  Refer to the
+documentation for details.  Use --without-perl to disable building
+PL/Perl." "$LINENO" 5
+  fi
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for flags to link embedded Perl" >&5
 $as_echo_n "checking for flags to link embedded Perl... " >&6; }
@@ -7537,6 +7541,33 @@ $as_echo "no" >&6; }
 fi
 
 
+
+  # We need libpython as a shared library.  In Python >=2.5, we asks
+  # Python directly.  But because this has been broken in Debian for a
+  # long time (http://bugs.debian.org/695979), and to support older
+  # Python versions, we see if there is a file that is named like a
+  # shared library as a fallback.
+
+  if test "$python_enable_shared" != 11; then
+# OS X does supply a .dylib even though Py_ENABLE_SHARED does not get set
+if test "$PORTNAME" = darwinX; then
+  python_enable_shared=1
+else
+  for dlsuffix in .so .dll .sl; do
+if ls "$python_libdir"/libpython*${dlsuffix}* >/dev/null 2>&1; then
+  python_enable_shared=1
+  break
+fi
+  done
+fi
+  fi
+
+  if test "$python_enable_shared" != 1; then
+as_fn_error $? "cannot build PL/Python because libpython is not a shared library
+You might have to rebuild your Python installation.  Refer to the
+documentation for details.  Use --without-python to disable building
+PL/Python." "$LINENO" 5
+  fi
 fi
 
 if test "$cross_compiling" = yes && test -z "$with_system_tzdata"; then
@@ -14736,12 +14767,15 @@ fi
 
 . "$TCL_CONFIG_SH"
 eval TCL_INCLUDE_SPEC=\"$TCL_INCLUDE_SPEC\"
-eval TCL_LIB_FILE=\"$TCL_LIB_FILE\"
 eval TCL_LIBS=\"$TCL_LIBS\"
 eval TCL_LIB_SPEC=\"$TCL_LIB_SPEC\"
 eval TCL_SHARED_BUILD=\"$TCL_SHARED_BUILD\"
 
-# now that we have TCL_INCLUDE_SPEC, we can check for 
+if test "$TCL_SHARED_BUILD" != 1; then
+  as_fn_error $? "cannot build PL/Tcl because Tcl is not a shared library
+Use --without-tcl to disable building PL/Tcl." "$LINENO" 5
+fi
+# now that we have TCL_INCLUDE_SPEC, we can check for 
 ac_save_CPPFLAGS=$CPPFLAGS
 CPPFLAGS="$TCL_INCLUDE_SPEC $CPPFLAGS"
 ac_fn_c_check_header_mongrel "$LINENO" "tcl.h" "ac_cv_header_tcl_h" "$ac_includes_default"
diff --git a/configure.in b/configure.in
index 1cd9e1e..a9e2257 100644
--- a/configure.in
+++ b/configure.in
@@ -889,12 +889,45 @@ if test "$with_perl" = yes; then
 AC_MSG_ERROR([Perl not found])
   fi
   PGAC_CHECK_PERL_CONFIGS([archlibexp,privlibexp,useshrplib])
+  if test "$perl_useshrplib" != yes && test "$perl_useshrplib" != true; then
+AC_MSG_ERROR([cannot bui

Re: [HACKERS] Reducing tuple overhead

[Amit Kapila]

On Thu, Apr 30, 2015 at 7:24 PM, Robert Haas  wrote:
>
> On Thu, Apr 30, 2015 at 9:46 AM, Amit Kapila 
wrote:
> > As the index expression contain table columns and all the functions
> > or operators used in expression must be IMMUTABLE, won't that
> > guarantee to avoid such a situation?
>
> The concern is that they might be labeled as immutable but not
> actually behave that way.
>
> The other, related problem is that the ordering operator might start
> to return different results than it did at index creation time.  For
> example, consider a btree index built on a text column.  Now consider
> 'yum update'.  glibc gets updated, collation ordering of various
> strings change, and now you've got tuples that are in the "wrong
> place" in the index, because when the index was built, we thought A <
> B, but now we think B < A.  You would think the glibc maintainers
> might avoid such changes in minor releases, or that the Red Hat guys
> would avoid packaging and shipping those changes in minor releases,
> but you'd be wrong.  We've seen cases where the master and the standby
> were both running RHEL X.Y (same X.Y on both servers) but they didn't
> agree on the collation definitions, so queries that worked on the
> master failed on the slave.
>

This is quite similar to IMMUTABLE function, but for an operator.  I think
guaranteeing the immutable nature for a function is the job of
application/user.
Oracle uses something similar (DETERMINISTIC functions) for function based
indexes and asks user to ensure the DETERMINISTIC property of function.
I think having synchronous delete (delete from index at the same time as
from heap) is one of the main differentiating factor for not having bloat in
indexes in some of the other databases.  If we don't want to change the
current property for functions or operators for indexes, then we can have a
new
type of index which can have visibility information and users are advised to
use such an index where they can ensure that functions or operators for
that index are IMMUTABLE.  Here, there is one argument that users might or
might not be able to ensure the same, but I think if it can be ensured by
users
of other successful databases, then the same should be possible for
PostgreSQL
users as well, after all this can bring a lot of value on table (avoiding
index bloat)
for users.


> > I think it will still
> > give us lot of benefit in more common cases.
>
> It's hard to figure out exactly what can work here.  Aside from
> correctness issues, the biggest problem with refinding the index
> tuples one-by-one and killing them is that it may suck for bulk
> operations.  When you delete one tuple from the table, refinding the
> index tuples and killing them immediately is probably the smartest
> thing you can do.  If you postpone it, somebody may be forced to split
> the page because it's full, whereas if you do it right away, the next
> guy who wants to put a tuple on that page may be able to make it fit.
> That's a big win.
>

Exactly, I have that big win in my mind.

> But if you want to delete 10 million tuples, doing an index scan for
> each one may induce a tremendous amount of random I/O on the index
> pages, possibly visiting and dirtying the same pages more than once.
> Scanning the whole index for tuples to kill avoids that.
>

Even doing it only from heap might not be cheap.
I think for doing BULK delete (as you described), users of other
databases have some smart ways like:
a.
If possible drop the indexes
b.
instead of deleting the records you don't want, SELECT OUT the
records you do -- drop the old table -- rename new table.
c.
Archive instead of delete Instead of deleting, just set a flag to mark
a row as archived, invalid, deleted, etc. This will avoid the immediate
overhead of index maintenance. Ignore the rows temporarily and let
some other job delete them later. The large downside to this is that it
affects any query on the table.
...
possibly some other ways.


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

Re: [HACKERS] initdb -S and tablespaces

[Abhijit Menon-Sen]

At 2015-04-30 16:56:17 -0700, t...@sss.pgh.pa.us wrote:
>
> As for the notion that this needs to be back-patched, I would say no.

Not even just the "fsync after crash" part? I could separate that out
from the control file changes and try to eliminate the duplication. I
think that would be worth back-patching, at least.

-- Abhijit


-- 
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] initdb -S and tablespaces

[Abhijit Menon-Sen]

At 2015-04-30 15:37:44 -0400, robertmh...@gmail.com wrote:
>
> 1. It doesn't do that.  As soon as we fsync the data directory, we
>reset the flag.  That's not what "ever disabled" means to me.

Could you suggest an acceptable alternative wording? I can't immediately
think of anything better than "disabled since the last restart". That is
conditional on our resetting the flag, which we will do only if fsync is
enabled at startup. So it's true, but not the whole truth.

> 2. I don't know why it's part of this patch.

In 20150115133245.gg5...@awork2.anarazel.de, Andres explained his
rationale as follows:

«What I am thinking of is that, currently, if you start the server
for initial loading with fsync=off, and then restart it, you're open
to data loss. So when the current config file setting is changed
from off to on, we should fsync the data directory. Even if there
was no crash restart.»

That's what I tried to implement.

> Also, it seems awfully unfortunate to me that we're duplicating a
> whole pile of code into xlog.c here.

I have pointed out and discussed the duplication several times. I did it
this way only because we were considering backporting the changes, and
at the time it seemed better to do this and fix the duplication in a
separate patch.

-- Abhijit


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

Re: [HACKERS] proposal: disallow operator "=>" and use it for named parameters

[Pavel Stehule]

It is done
Dne 1.5.2015 3:11 napsal uživatel "Bruce Momjian" :

> On Tue, Mar 10, 2015 at 02:51:30PM -0400, Robert Haas wrote:
> > On Tue, Mar 10, 2015 at 2:32 PM, Pavel Stehule 
> wrote:
> > > 1. funcname_signature_string
> > > 2. get_rule_expr
> >
> > Thanks.  Patch attached.  I'll commit this if there are no objections.
>
> Robert, are you going to apply this?
>
> --
>   Bruce Momjian  http://momjian.us
>   EnterpriseDB http://enterprisedb.com
>
>   + Everyone has their own god. +
>

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Fri, May 1, 2015 at 1:38 AM, Robert Haas  wrote:
> On Thu, Apr 30, 2015 at 9:15 AM, Sawada Masahiko  
> wrote:
>> On Thu, Apr 30, 2015 at 8:39 PM, Robert Haas  wrote:
>>> On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko  
>>> wrote:
 Attached v10 patch is latest version patch.
 The syntax is,
 REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]

 That is, WITH clause is optional.
>>>
>>> I thought we agreed on moving this earlier in the command:
>>>
>>> http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
>>>
>>
>> Oh, I see.
>> Attached patch is modified syntax as
>> REINDEX [VERBOSE] { INDEX | ... } name
>>
>> Thought?
>
> I thought what we agreed on was:
>
> REINDEX (flexible options) { INDEX | ... } name
>
> The argument wasn't about whether to use flexible options, but where
> in the command to put them.
>

VACUUM has both syntax: with parentheses and without parentheses.
I think we should have both syntax for REINDEX like VACUUM does
because it would be pain to put parentheses whenever we want to do
REINDEX.
Are the parentheses optional in REINDEX command?

And CLUSTER should have syntax like that in future?

Regards,

---
Sawada Masahiko


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

Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0

[Heikki Linnakangas]

On 04/27/2015 11:02 PM, Peter Geoghegan wrote:

On Mon, Apr 27, 2015 at 8:31 PM, Heikki Linnakangas  wrote:

I thought we had an ironclad scheme to prevent deadlocks like this, so I'd
like to understand why that happens.



Okay. I think I know how it happens (I was always skeptical of the
idea that this would be 100% reliable), but I'll be able to show you
exactly how tomorrow. I'll have pg_xlogdump output then.


I was able to reproduce this, using two sessions, so that on session 
does a regular INSERT, and another does INSERT ON CONFLICT, after adding 
a sleep(5) to a strategic place. So this was indeed a live bug, 
reproducible even without the hack you had to allow ON CONFLICT UPDATE 
with exclusion constraints. Fortunately this is easy to fix.


Here's how to reproduce:

1. Insert "sleep(5)" into ExecInsertIndexTuples, just after the 
index_insert() call.


2. Create the test table and index:

create extension btree_gist;
create table foo (id int4, constraint foo_x exclude using gist (id with 
=) );


3. Launch two psql sessions, A and B. Do the following:

A: set deadlock_timeout='10s';
B: set deadlock_timeout='20s';
A: begin; select txid_current();
B: begin; select txid_current();

A: insert into foo values (1) on conflict do nothing;
(the insert will hit the sleep(5) - quickly perform the second insert 
quickly: )

B: insert into foo values (1);

At this point, both transactions have already inserted the tuple to the 
heap. A has done so speculatively, but B has done a regular insert. B 
will find A's tuple and wait until A's speculative insertion completes. 
A will find B's tuple, and wait until B completes, and you get the 
deadlock. Thanks to the way the deadlock_timeout's are set, A will 
detect the deadlock first and abort. That's not cool with ON CONFLICT 
IGNORE.


To fix that, we need to fix the "livelock insurance" check so that A 
does not wait for B here. Because B is not a speculative insertion, A 
should cancel its speculative insertion and retry instead. (I pushed the 
one-line fix for that to your github repository)


- 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] procost for to_tsvector

[Bruce Momjian]

On Wed, Mar 11, 2015 at 02:40:16PM +, Andrew Gierth wrote:
> An issue that comes up regularly on IRC is that text search queries,
> especially on relatively modest size tables or for relatively
> non-selective words, often misplan as a seqscan based on the fact that
> to_tsvector has procost=1.
> 
> Clearly this cost number is ludicrous.
> 
> Getting the right cost estimate would obviously mean taking the cost of
> detoasting into account, but even without doing that, there's a strong
> argument that it should be increased to at least the order of 100.
> (With the default cpu_operator_cost that would make each to_tsvector
> call cost 0.25.)
> 
> (The guy I was just helping on IRC was seeing a slowdown of 100x from a
> seqscan in a query that selected about 50 rows from about 500.)

Where are we on setting increasing procost for to_tsvector?

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

  + Everyone has their own god. +


-- 
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: adaptive ndistinct estimator v4

[Tomas Vondra]

On 05/01/15 00:18, Robert Haas wrote:

On Thu, Apr 30, 2015 at 5:31 PM, Heikki Linnakangas  wrote:


You can override the ndistinct estimate with ALTER TABLE. I think
that's enough for an escape hatch.


I'm not saying that isn't nice to have, but I don't think it really
helps much here. Setting the value manually requires that you know
what value to set, and you might not. If, on some workloads, the old
algorithm beats the new one reliably, you want to be able to
actually go back to the old algorithm, not manually override every
wrong decision it makes. A GUC for this is pretty cheap insurance.


IMHO this is exactly the same situation as with the current ndistinct 
estimator. If we find out we'd have to use this workaround more 
frequently than before, then clearly the new estimator is rubbish and 
should not be committed.


In other words, I agree with Heikki.


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


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

Re: [HACKERS] PATCH: adaptive ndistinct estimator v4

[Tomas Vondra]

Hi,

On 04/30/15 22:57, Robert Haas wrote:

On Tue, Mar 31, 2015 at 3:02 PM, Tomas Vondra
 wrote:

attached is v4 of the patch implementing adaptive ndistinct estimator.


So, I took a look at this today. It's interesting work, but it looks
more like a research project than something we can commit to 9.5. As
far as I can see, this still computes the estimate the way we do
today, but then also computes the estimate using this new method.
The estimate computed the new way isn't stored anywhere, so this
doesn't really change behavior, except for printing out a WARNING
comparing the values produced by the two estimators.


I agree that this is not ready for 9.5 - it was meant as an experiment
(hence printing the estimate in a WARNING, to make it easier to compare
the value to the current estimator). Without that it'd be much more
complicated to compare the old/new estimates, but you're right this is
not suitable for commit.

So far it received only reviews from Jeff Janes (thanks!), but I think a 
change like this really requires a more thorough review, including the 
math part. I don't expect that to happen at the very end of the last CF 
before the freeze.



IMHO, the comments in this patch are pretty inscrutable.  I believe
this is because they presume more knowledge of what the patch is doing
than I myself possess.  For example:

+ * The AEE estimator is based on solving this equality (for "m")
+ *
+ * m - f1 - f2 = f1 * (A + A(m)) / (B + B(m))
+ *
+ * where A, B are effectively constants (not depending on m), and A(m)
+ * and B(m) are functions. This is equal to solving
+ *
+ * 0 = f1 * (A + A(m)) / (B + B(m)) - (m - f1 - f2)

Perhaps I am just a dummy, but I have no idea what any of that means.
I think that needs to be fixed so that someone who knows what
n_distinct is but knows nothing about the details of these estimators
can get an idea of how they are doing their thing without too much
effort.  I think a lot of the comments share this problem.


Well, I don't think you're dummy, but this requires reading the paper 
describing the estimator. Explaining that fully would essentially mean 
copying a large portion of the paper in the comment, and I suppose 
that's not a good idea. The explanation might be perhaps a bit more 
detailed, though - not sure what's the right balance.



Aside from the problems mentioned above, there's the broader question
of how to evaluate the quality of the estimates produced by this
estimator vs. what we're doing right now. I see that Jeff Janes has
pointed out some cases that seem to regress with this patch; those
presumably need some investigation, or at least some comment. And I
think some testing from other people would be good too, before we
commit to this.


Yeah, evaluating is difficult. I think that Jeff's approach - i.e. 
testing the estimator on real-world data sets - is the right approach 
here. Testing on synthetic data sets has it's value too (if only to 
better understand the failure cases).



Leaving that aside, at some point, you'll say, "OK, there may be some
regressions left but overall I believe this is going to be a win in
most cases". It's going to be really hard for anyone, no matter how
smart, to figure out through code review whether that is true. So
committing this figures to be extremely frightening. It's just not
going to be reasonably possible to know what percentage of users are
going to be more happy after this change and what percentage are
going to be less happy.


For every pair of estimators you can find cases where one of them is 
better than the other one. It's pretty much impossible to find an 
estimator that beats all other estimators on all possible inputs.


There's no way to make this an improvement for everyone - it will 
produce worse estimates in some cases, and we have to admit that. If we 
think this is unacceptable, we're effectively stuck with the current 
estimator forever.



Therefore, I think that:

1. This should be committed near the beginning of a release cycle,
not near the end. That way, if there are problem cases, we'll have a
year or so of developer test to shake them out.


+1


2. There should be a compatibility GUC to restore the old behavior.
The new behavior should be the default, because if we're not
confident that the new behavior will be better for most people, we
have no business installing it in the first place (plus few people
will try it). But just in case it turns out to suck for some people,
we should provide an escape hatch, at least for a few releases.


I think a "compatibility GUC" is a damn poor solution, IMNSHO.

For example, GUCs are database-wide, but I do expect the estimator to 
perform worse only on a few data sets / columns. So making this a 
column-level settings would be more appropriate, I think.


But it might work during the development cycle, as it would make 
comparing the estimators possible (just run the tests with the GUC set 
differently). Assuming we'll re-evaluate it 

Re: [HACKERS] Final Patch for GROUPING SETS

[Noah Misch]

On Thu, Apr 30, 2015 at 05:35:26AM +0100, Andrew Gierth wrote:
> > "Andres" == Andres Freund  writes:

>  >> + * TODO: AGG_HASHED doesn't support multiple grouping sets yet.
> 
>  Andres> Are you intending to resolve this before an eventual commit?
> 
> Original plan was to tackle AGG_HASHED after a working implementation
> was committed;

+1 for that plan.

>  Andres> Possibly after the 'structural' issues are resolved? Or do you
>  Andres> think this can safely be put of for another release?
> 
> I think the feature is useful even without AGG_HASHED, even though that
> means it can sometimes be beaten on performance by using UNION ALL of
> many separate GROUP BYs; but I'd defer to the opinions of others on that
> point.

It will be a tough call, and PostgreSQL has gone each way on some recent
feature.  I recommend considering both GroupAggregate and HashAggregate in all
design discussion but continuing to work toward a first commit implementing
GroupAggregate alone.  With that in the tree, we'll be in a better position to
decide whether to release a feature paused at that stage in its development.
Critical facts are uncertain, so a discussion today would be unproductive.

>  Andres> So, having quickly scanned through the patch, do I understand
>  Andres> correctly that the contentious problems are:
> 
>  Andres> * Arguably this converts the execution *tree* into a DAG. Tom
>  Andres> seems to be rather uncomfortable with that. I am wondering
>  Andres> whether this really is a big deal - essentially this only
>  Andres> happens in a relatively 'isolated' part of the tree right?
>  Andres> I.e. if those chained together nodes were considered one node,
>  Andres> there would not be any loops?  Additionally, the way
>  Andres> parametrized scans works already essentially "violates" the
>  Andres> tree paradigma somewhat.

I agree with your assessment.  That has been contentious.

> The major downsides as I see them with the current approach are:
> 
> 1. It makes plans (and hence explain output) nest very deeply if you
> have complex grouping sets (especially cubes with high dimensionality).
> 
> This can make explain very slow in the most extreme cases

I'm not worried about that.  If anything, the response is to modify explain to
more-quickly/compactly present affected plan trees.

> 2. A union-based approach would have a chance of including AGG_HASHED
> support without any significant code changes,

>   -> CTE x
>  -> entire input subplan here
>   -> Append
>  -> GroupAggregate
> -> Sort
>-> CTE Scan x
>  -> GroupAggregate
> -> Sort
>-> CTE Scan x
>  -> HashAggregate
> -> CTE Scan x
>  [...]

This uses 50-67% more I/O than the current strategy, which makes it a dead end
from my standpoint.  Details:
http://www.postgresql.org/message-id/20141221210005.ga1864...@tornado.leadboat.com

>  Andres> Are those the two bigger controversial areas? Or are there
>  Andres> others in your respective views?

> Another controversial item was the introduction of GroupedVar.

I know of no additional controversies to add to this list.

Thanks,
nm


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

Re: [HACKERS] proposal: disallow operator "=>" and use it for named parameters

[Bruce Momjian]

On Tue, Mar 10, 2015 at 02:51:30PM -0400, Robert Haas wrote:
> On Tue, Mar 10, 2015 at 2:32 PM, Pavel Stehule  
> wrote:
> > 1. funcname_signature_string
> > 2. get_rule_expr
> 
> Thanks.  Patch attached.  I'll commit this if there are no objections.

Robert, are you going to apply this?

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

  + Everyone has their own god. +


-- 
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] transforms vs. CLOBBER_CACHE_ALWAYS

[Christian Ullrich]

* Andrew Dunstan:


friarbird is a FreeBSD buildfarm animal running with
-DCLOBBER_CACHE_ALWAYS. It usually completes a run in about 6.5 hours.
However, it's been stuck since Monday running the plpython regression
tests. The only relevant commit seems to be the transforms feature.
Here's what it's been doing:



query| SELECT cursor_plan();


Same here, on jaguarundi. I actually killed it intentionally this 
morning, hoping that whatever the problem was might have been fixed 
already. No such luck.


I would suspect that it might have something to do with the OS, if all 
the other CCA animals weren't lining up nicely behind in the buildfarm 
status page.



I imagine it was in some sort of infinite loop. gdb says it's all in
src/backend/utils/cache/plancache.c, although not the same line each
time I run it.


I ktrace'd it this morning, but cleverly did not keep the dump. It 
looked much the same to me, though, it was reading the same filenode 
over and over again.


--
Christian




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

[HACKERS] BuildTupleFromCStrings Memory Documentation?

[Jason Petersen]

Within the core codebase, BuildTupleFromCStrings is often called within a temporary memory context cleared after the call. In dblink.c, this is justified as being needed to “[clean up] not only the data we have direct access to, but any cruft the I/O functions might leak”.I wrote a pretty minimal case to call BuildTupleFromCStrings in a loop (attached) and found that I was using 40GB of RAM in a few minutes, though I was not allocating any memory myself and immediately freed the tuple it returned.Is the need to wrap this call in a protective context documented anywhere? Portions of the documentation use BuildTupleFromCStrings in examples without mentioning this precaution. Is it just well-known, or did I miss a README or comment somewhere?
--Jason PetersenSoftware Engineer | Citus Data303.736.9255ja...@citusdata.com



tup_memleak.c
Description: Binary data


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [HACKERS] One question about security label command

[Kohei KaiGai]

2015-05-01 7:40 GMT+09:00 Alvaro Herrera :
> Kouhei Kaigai wrote:
>> > * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> > > The idea of making the regression test entirely independent of the
>> > > system's policy would presumably solve this problem, so I'd kind of
>> > > like to see progress on that front.
>> >
>> > Apologies, I guess it wasn't clear, but that's what I was intending to
>> > advocate.
>> >
>> OK, I'll try to design a new regression test policy that is independent
>> from the system's policy assumption, like unconfined domain.
>>
>> Please give me time for this work.
>
> Any progress here?
>
Not done.
The last version I rebuild had a trouble on user/role transition from
unconfined_u/unconfined_r to the self defined user/role...
So, I'm trying to keep the user/role field (that is not redefined for
several years) but to define self domain/types (that have been
redefined multiple times) for the regression test at this moment.

Thanks,
-- 
KaiGai Kohei 


-- 
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] Use outerPlanState() consistently in executor code

[Tom Lane]

Robert Haas  writes:
> Yeah, that seems fine.  Anyone want to object to this?

This hunk:

@@ -299,6 +301,7 @@ ExecReScanSort(SortState *node)
return;

/* must drop pointer to sort result tuple */
+   outerPlan = outerPlanState(node);
ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);

/*

seems to have involved throwing darts at the source code to decide where
to insert the variable initialization; certainly putting a totally
unrelated operation between a comment and the line it describes is not
an improvement to code clarity in my book.

I think I'd have done many of these as

+   PlanState   *outerPlan = outerPlanState(node);

rather than finding assorted random places to initialize the variables.

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] initdb -S and tablespaces

[Tom Lane]

Robert Haas  writes:
> On Thu, Apr 30, 2015 at 6:44 PM, Alvaro Herrera
>  wrote:
>> Ah, so that's not the duplicate code that I was remembering -- I think
>> it's walkdir() or something like that, which is in initdb IIRC.

> Yeah, walkdir() is there too.  But if we're going to add that to the
> backend, I think it should go in src/backend/storage/file, not
> src/backend/access/transam.

Agreed that .../transam is a pretty horrid choice; but maybe we should
be thinking about putting it in src/common, so there's only one copy?

As for the notion that this needs to be back-patched, I would say no.

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] Use outerPlanState() consistently in executor code

[Robert Haas]

On Thu, Apr 30, 2015 at 1:44 PM, Qingqing Zhou
 wrote:
> On Thu, Apr 30, 2015 at 8:02 AM, Robert Haas  wrote:
>> I don't mind the MSDOS newlines, but the UTF-16le bit is inconvenient.
>> UTF-8 would be much better, so I don't have to figure out how to
>> convert.
>>
>
> The patch is generated via github windows tool and that's possibly
> why. I regenerated it in Linux box and see attached (sending this
> email in Windows and I hope no magic happens in-between).
>
> Please let me know if that works.

Yeah, that seems fine.  Anyone want to object to this?

-- 
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] initdb -S and tablespaces

[Robert Haas]

On Thu, Apr 30, 2015 at 6:44 PM, Alvaro Herrera
 wrote:
> Robert Haas wrote:
>> On Thu, Apr 30, 2015 at 6:18 PM, Alvaro Herrera
>>  wrote:
>> >> Also, it seems awfully unfortunate to me that we're duplicating a
>> >> whole pile of code into xlog.c here.  Maybe there's no way to avoid
>> >> the code duplication, but pre_sync_fname() seems like it'd more
>> >> naturally go in fd.c than here.  I dunno where walkdir should go, but
>> >> again, not in xlog.c.
>> >
>> > Hm, there's an interest in backpatching this as a bugfix, if I
>> > understand correctly; hence the duplicated code.  We could remove the
>> > duplicity later with a refactoring patch in master only.
>>
>> That seems pretty silly.  If we going to add pre_sync_fname() to every
>> branch, we should add it to the same (correct) file in all of them,
>> not put it in xlog.c in the back-branches and fd.c in master.
>
> Ah, so that's not the duplicate code that I was remembering -- I think
> it's walkdir() or something like that, which is in initdb IIRC.

Yeah, walkdir() is there too.  But if we're going to add that to the
backend, I think it should go in src/backend/storage/file, not
src/backend/access/transam.

-- 
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] Broken handling of NULLs in TG_ARGV

[Tom Lane]

Jim Nasby  writes:
> plpgsql's handling of NULLs in TG_ARGV turns actual nulls into text 
> 'null'. Hopefully we can all agree that's broken.

You apparently have not read the CREATE TRIGGER reference page very
carefully:

arguments

An optional comma-separated list of arguments to be
provided to the function when the trigger is executed. The
arguments are literal string constants. Simple names and
numeric constants can be written here, too, but they will
all be converted to strings.

There isn't any such thing as a genuine SQL NULL argument; the examples
you provided are just text strings, not SQL NULLs.  In order to make them
be actual nulls, we would have to redefine the arguments as being
expressions of some sort, which is problematic for backwards-compatibility
reasons.  It also seems like rather a lot of new mechanism to add for
something with (evidently) near-zero user demand.

regards, tom lane


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

[HACKERS] Broken handling of NULLs in TG_ARGV

[Jim Nasby]

plpgsql's handling of NULLs in TG_ARGV turns actual nulls into text 
'null'. Hopefully we can all agree that's broken. I'd like to fix it, 
but wonder how to handle existing user code.


My suspicion is that most users will never notice this and I can just 
fix it.


I could also add a plpgsql option to provide the old behavior.

What I'd prefer not to do is keep defaulting to the current behavior. 
Users are unlikely to notice that, keep using the broken behavior, and 
still complain when we change the default.


decibel@decina.local=# create temp table t(t text);
CREATE TABLE
decibel@decina.local=# create function tg() returns trigger language 
plpgsql as $$BEGIN RAISE '"%" is null? %', tg_argv[0], tg_argv[0] is 
null; end$$;

CREATE FUNCTION
decibel@decina.local=# create trigger t before insert on t for each row 
execute procedure tg(NULL);

CREATE TRIGGER
decibel@decina.local=# insert into t values('a');
ERROR:  "null" is null? f
decibel@decina.local=# drop trigger t on t;
DROP TRIGGER
decibel@decina.local=# create trigger t before insert on t for each row 
execute procedure tg();

CREATE TRIGGER
decibel@decina.local=# insert into t values('a');
ERROR:  "" is null? t
decibel@decina.local=# drop trigger t on t;
DROP TRIGGER
decibel@decina.local=# create trigger t before insert on t for each row 
execute procedure tg('null');

CREATE TRIGGER
decibel@decina.local=# insert into t values('a');
ERROR:  "null" is null? f
decibel@decina.local=#

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] collations in shared catalogs?

[Bruce Momjian]

On Thu, Apr 30, 2015 at 08:16:09AM -0700, Tom Lane wrote:
> Bruce Momjian  writes:
>  On 2015-02-25 12:08:32 -0500, Tom Lane wrote:
> > The most obvious fix is to change "provider" to a NAME column.
> 
> > Where are we on this?
> 
> Not done yet, but we should make a point of making that fix before 9.5.
> Please add it to the open items page for 9.5.
> 
> I am not sure there's anything useful to be done about this in the back
> branches.

Done.

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

  + Everyone has their own god. +


-- 
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] initdb -S and tablespaces

[Alvaro Herrera]

Robert Haas wrote:
> On Thu, Apr 30, 2015 at 6:18 PM, Alvaro Herrera
>  wrote:
> >> Also, it seems awfully unfortunate to me that we're duplicating a
> >> whole pile of code into xlog.c here.  Maybe there's no way to avoid
> >> the code duplication, but pre_sync_fname() seems like it'd more
> >> naturally go in fd.c than here.  I dunno where walkdir should go, but
> >> again, not in xlog.c.
> >
> > Hm, there's an interest in backpatching this as a bugfix, if I
> > understand correctly; hence the duplicated code.  We could remove the
> > duplicity later with a refactoring patch in master only.
> 
> That seems pretty silly.  If we going to add pre_sync_fname() to every
> branch, we should add it to the same (correct) file in all of them,
> not put it in xlog.c in the back-branches and fd.c in master.

Ah, so that's not the duplicate code that I was remembering -- I think
it's walkdir() or something like that, which is in initdb IIRC.

-- 
Á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] One question about security label command

[Alvaro Herrera]

Stephen Frost wrote:

Hi,

> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:

> > Could you provide a buildfarm animal that runs the sepgsql test in all
> > branches on a regular basis?
> 
> Would be great if KaiGai can, of course, but I'm planning to stand one
> up here soon in any case.

I don't think this is done, is it?

-- 
Á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] One question about security label command

[Alvaro Herrera]

Kouhei Kaigai wrote:
> > * Tom Lane (t...@sss.pgh.pa.us) wrote:
> > > The idea of making the regression test entirely independent of the
> > > system's policy would presumably solve this problem, so I'd kind of
> > > like to see progress on that front.
> > 
> > Apologies, I guess it wasn't clear, but that's what I was intending to
> > advocate.
> >
> OK, I'll try to design a new regression test policy that is independent
> from the system's policy assumption, like unconfined domain.
> 
> Please give me time for this work.

Any progress here?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

[HACKERS] Implementing SQL ASSERTION

[Joe Wildish]

Hi all,

I’m wondering if there are other people out there working on implementing SQL 
ASSERTION functionality?

I’ve recently spent a bit of time looking to implement the execution models 
described in “Applied Mathematics for Database Professionals” by Toon 
Koppelaars and Lex de Haan.  I’ve gotten as far as execution model 3 and am now 
looking at deriving polarity of involved tables to do EM4 (described in some 
detail in “Deriving Production Rules for Constraint Maintenance”, Ceri & Widom, 
VLDB Conference 1990, p555-577). EM5 & EM6 look rather more difficult but I’m 
intending to try and implement those, too.

If there are other people working on this stuff it would be great to 
collaborate.

Regards.
-Joe

-- 
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] initdb -S and tablespaces

[Robert Haas]

On Thu, Apr 30, 2015 at 6:18 PM, Alvaro Herrera
 wrote:
>> Also, it seems awfully unfortunate to me that we're duplicating a
>> whole pile of code into xlog.c here.  Maybe there's no way to avoid
>> the code duplication, but pre_sync_fname() seems like it'd more
>> naturally go in fd.c than here.  I dunno where walkdir should go, but
>> again, not in xlog.c.
>
> Hm, there's an interest in backpatching this as a bugfix, if I
> understand correctly; hence the duplicated code.  We could remove the
> duplicity later with a refactoring patch in master only.

That seems pretty silly.  If we going to add pre_sync_fname() to every
branch, we should add it to the same (correct) file in all of them,
not put it in xlog.c in the back-branches and fd.c in master.

> However, now that you mention a pg_control flag, it becomes evident to
> me that a change to pg_control cannot be back-patched ...

Indeed.  But I think we can solve that problem by just ripping that
part out.  Unless I'm missing something, it's not really doing
anything useful.

-- 
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] PATCH: adaptive ndistinct estimator v4

[Robert Haas]

On Thu, Apr 30, 2015 at 5:31 PM, Heikki Linnakangas  wrote:
> On 04/30/2015 01:57 PM, Robert Haas wrote:
>> 2. There should be a compatibility GUC to restore the old behavior.
>> The new behavior should be the default, because if we're not confident
>> that the new behavior will be better for most people, we have no
>> business installing it in the first place (plus few people will try
>> it).  But just in case it turns out to suck for some people, we should
>> provide an escape hatch, at least for a few releases.
>
> You can override the ndistinct estimate with ALTER TABLE. I think that's
> enough for an escape hatch.

I'm not saying that isn't nice to have, but I don't think it really
helps much here.  Setting the value manually requires that you know
what value to set, and you might not.  If, on some workloads, the old
algorithm beats the new one reliably, you want to be able to actually
go back to the old algorithm, not manually override every wrong
decision it makes.  A GUC for this is pretty cheap insurance.

-- 
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] initdb -S and tablespaces

[Alvaro Herrera]

Robert Haas wrote:

> Also, it seems awfully unfortunate to me that we're duplicating a
> whole pile of code into xlog.c here.  Maybe there's no way to avoid
> the code duplication, but pre_sync_fname() seems like it'd more
> naturally go in fd.c than here.  I dunno where walkdir should go, but
> again, not in xlog.c.

Hm, there's an interest in backpatching this as a bugfix, if I
understand correctly; hence the duplicated code.  We could remove the
duplicity later with a refactoring patch in master only.

However, now that you mention a pg_control flag, it becomes evident to
me that a change to pg_control cannot be back-patched ...

-- 
Á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] PATCH: adaptive ndistinct estimator v4

[Heikki Linnakangas]

On 04/30/2015 01:57 PM, Robert Haas wrote:


2. There should be a compatibility GUC to restore the old behavior.
The new behavior should be the default, because if we're not confident
that the new behavior will be better for most people, we have no
business installing it in the first place (plus few people will try
it).  But just in case it turns out to suck for some people, we should
provide an escape hatch, at least for a few releases.


You can override the ndistinct estimate with ALTER TABLE. I think that's 
enough for an escape hatch.


- 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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Kouhei Kaigai]

> On Thu, Apr 30, 2015 at 9:16 AM, Kouhei Kaigai  wrote:
> > It seems to me the code block for T_ForeignScan and T_CustomScan in
> > setrefs.c are a bit large. It may be better to have a separate
> > function like T_IndexOnlyScan.
> > How about your opinion?
> 
> Either way is OK with me.  Please do as you think best.
>
OK, in setrefs.c, I moved the code block for T_ForeignScan and T_CustomScan
into set_foreignscan_references() and set_customscan_references() for each.
Its nest-level is a bit deep to keep all the stuff within 80-characters row.
It also uses bms_add_member(), instead of bms_shift_members() reverted.

> >> +* Sanity check. Pseudo scan tuple-descriptor shall be constructed
> >> +* based on the fdw_ps_tlist, excluding resjunk=true, so we need to
> >> +* ensure all valid TLEs have to locate prior to junk ones.
> >>
> >> Is the goal here to make attribute numbers match up?  If so, between
> >> where and where?  If not, please explain further.
> >>
> > No, its purpose is to reduce unnecessary projection.
> >
> > The *_ps_tlist is not only used to construct tuple-descriptor of
> > Foreign/CustomScan with scanrelid==0, but also used to resolve var-
> > nodes with varno==INDEX_VAR in EXPLAIN command.
> >
> > For example,
> >   SELECT t1.y, t2.b FROM t1, t2 WHERE t1.x = t2.a;
> >
> > If "t1.x = t2.a" is executable on external computing resource (like
> > remote RDBMS or GPU device, etc), both of t1.x and t2.a don't need
> > to appear on the targetlist of joinrel.
> > In this case, the best *_ps_tlist consists of two var-nodes of t1.x
> > and t2.a because it fits tuple-descriptor of result tuple slot, thus
> > it can skip per-tuple projection.
> >
> > On the other hands, we may want to print out expression clause that
> > shall be executed on the external resource; "t1.x = t2.a" in this
> > case. If FDW/CSP keeps this clause in expression form, its var-nodes
> > shall be rewritten to a pair of INDEX_VAR and resno on *_ps_tlist.
> > So, deparse_expression() needs to be capable to find out "t1.x" and
> > "t2.a" on the *_ps_tlist. However, it does not make sense to include
> > these variables on the scan tuple-descriptor.
> >
> > ExecInitForeignScan() and ExecInitCustomScan() makes its scan tuple-
> > descriptor using ExecCleanTypeFromTL(), not ExecTypeFromTL(), to omit
> > these unreferenced variables on the *_ps_tlist. All the var-nodes with
> > INDEX_VAR shall be identified by offset from head of the list, we cannot
> > allow any target-entry with resjunk=false after ones with resjunk=true,
> > to keep the expected varattno.
> >
> > This sanity checks ensures no target-entry with resjunk=false after
> > the resjunk=true. It helps to distinct attributes to be included in
> > the result tuple from the ones for just reference in EXPLAIN.
> >
> > Did my explain above introduced the reason of this sanity check well?
> 
> Yeah, I think so.  So what we want to do in this comment is summarize
> all of that briefly.  Maybe something like this:
> 
> "Sanity check.  There may be resjunk entries in fdw_ps_tlist that are
> included only to help EXPLAIN deparse plans properly.  We require that
> these are at the end, so that when the executor builds the scan
> descriptor based on the non-junk entries, it gets the attribute
> numbers correct."
>
Thanks, I used this sentence as is.

> >> +   if (splan->scan.scanrelid == 0)
> >> +   {
> >> ...
> >> +   }
> >> splan->scan.scanrelid += rtoffset;
> >>
> >> Does this need an "else"?  It seems surprising that you would offset
> >> scanrelid even if it's starting out as zero.
> >>
> >> (Note that there are two instances of this pattern.)
> >>
> > 'break' was put on the tail of if-block, however, it may lead potential
> > bugs in the future. I'll use if-else manner as usual.
> 
> Ah, OK, I missed that.  Yeah, that's probably a good change.
>
set_foreignscan_references() and set_customscan_references() are
split by two portions using the manner above; a code block if scanrelid==0
and others.

> I assume you realize you did not attach an updated patch?
>
I wanted to submit the v14 after the above items get clarified.
The attached patch (v14) includes all what you suggested in the previous
message.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 



pgsql-v9.5-custom-join.v14.patch
Description: pgsql-v9.5-custom-join.v14.patch

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

Re: [HACKERS] PATCH: adaptive ndistinct estimator v4

[Robert Haas]

On Tue, Mar 31, 2015 at 3:02 PM, Tomas Vondra
 wrote:
> attached is v4 of the patch implementing adaptive ndistinct estimator.

So, I took a look at this today.  It's interesting work, but it looks
more like a research project than something we can commit to 9.5.  As
far as I can see, this still computes the estimate the way we do
today, but then also computes the estimate using this new method.  The
estimate computed the new way isn't stored anywhere, so this doesn't
really change behavior, except for printing out a WARNING comparing
the values produced by the two estimators.

IMHO, the comments in this patch are pretty inscrutable.  I believe
this is because they presume more knowledge of what the patch is doing
than I myself possess.  For example:

+ * The AEE estimator is based on solving this equality (for "m")
+ *
+ * m - f1 - f2 = f1 * (A + A(m)) / (B + B(m))
+ *
+ * where A, B are effectively constants (not depending on m), and A(m)
+ * and B(m) are functions. This is equal to solving
+ *
+ * 0 = f1 * (A + A(m)) / (B + B(m)) - (m - f1 - f2)

Perhaps I am just a dummy, but I have no idea what any of that means.
I think that needs to be fixed so that someone who knows what
n_distinct is but knows nothing about the details of these estimators
can get an idea of how they are doing their thing without too much
effort.  I think a lot of the comments share this problem.

Aside from the problems mentioned above, there's the broader question
of how to evaluate the quality of the estimates produced by this
estimator vs. what we're doing right now.  I see that Jeff Janes has
pointed out some cases that seem to regress with this patch; those
presumably need some investigation, or at least some comment.  And I
think some testing from other people would be good too, before we
commit to this.

Leaving that aside, at some point, you'll say, "OK, there may be some
regressions left but overall I believe this is going to be a win in
most cases".  It's going to be really hard for anyone, no matter how
smart, to figure out through code review whether that is true.  So
committing this figures to be extremely frightening.  It's just not
going to be reasonably possible to know what percentage of users are
going to be more happy after this change and what percentage are going
to be less happy.

Therefore, I think that:

1. This should be committed near the beginning of a release cycle, not
near the end.  That way, if there are problem cases, we'll have a year
or so of developer test to shake them out.

2. There should be a compatibility GUC to restore the old behavior.
The new behavior should be the default, because if we're not confident
that the new behavior will be better for most people, we have no
business installing it in the first place (plus few people will try
it).  But just in case it turns out to suck for some people, we should
provide an escape hatch, at least for a few releases.

3. There should be some clear documentation in the comments indicating
why we believe that this is a whole lot better than what we do today.
Maybe this has been discussed adequately on the thread and maybe it
hasn't, but whoever goes to look at the committed code should not have
to go root through hackers threads to understand why we replaced the
existing estimator.  It should be right there in the code.  If,
hypothetically speaking, I were to commit this, and if, again strictly
hypothetically, another distinguished committer were to write back a
year or two later, "clearly Robert was an idiot to commit this because
it's no better than what we had before" then I want to be able to say
"clearly, you have not what got committed very carefully, because the
comment for function  clearly explains that this new technology
is teh awesome".

-- 
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] [PATCH] HINT: pg_hba.conf changed since last config reload

[Peter Eisentraut]

On 12/20/14 12:11 PM, Steve Singer wrote:
> On 12/19/2014 10:41 AM, Alex Shulgin wrote:
>> I don't think so.  The scenario this patch relies on assumes that the
>> DBA will remember to look in the log if something goes wrong, and in
>> your case there would be a message like the following:
>>
>> WARNING:  pg_hba.conf not reloaded
>>
>> So an extra hint about file timestamp is unneeded.
> 
> That makes sense to me.
> I haven't found any new issues with this patch.
> I think it is ready for a committer.

There were later comments in this thread that disagreed with the extra
logging infrastructure, and there were some questions about whether it
should only log on failed authentication attempts.  Altogether, still
some open questions about behavior and implementation approach.  So I'm
marking this as returned with feedback for now.

Personally, I think this could be a useful feature, but it needs more
fine-tuning.




-- 
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] pgbench -f and vacuum

[Robert Haas]

On Thu, Apr 30, 2015 at 4:17 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes  wrote:
>>> But as far as what has been discussed on the central topic of this thread, I
>>> think that doing the vacuum and making the failure for non-existent tables
>>> be non-fatal when -f is provided would be an improvement.  Or maybe just
>>> making it non-fatal at all times--if the table is needed and not present,
>>> the session will fail quite soon anyway.  I don't see the other changes as
>>> being improvements.  I would rather just learn to add the -n when I use -f
>>> and don't have the default tables in place, than have to learn new methods
>>> for saying "no really, I left -n off on purpose" when I have a custom file
>>> which does use the default tables and I want them vacuumed.
>
>> So, discussion seems to have died off here.  I think what Jeff is
>> proposing here is a reasonable compromise.  Patch for that attached.
>
> +1 as to the basic behavior, but I'm not convinced that this is
> user-friendly reporting:
>
> +   if (PQresultStatus(res) != PGRES_COMMAND_OK)
> +   fprintf(stderr, "%s", PQerrorMessage(con));
>
> I would be a bit surprised to see pgbench report an ERROR and then
> continue on anyway; I might think that was a bug, even.  I am not
> sure exactly what it should print instead though.  Some perhaps viable
> proposals:
>
> * don't print anything at all, just chug along.
>
> * do something like
> fprintf(stderr, "Ignoring: %s", PQerrorMessage(con));
>
> * add something like "(Ignoring this error and continuing anyway)"
>   on a line after the error message.
>
> (I realize this takes us right back into the bikeshedding game, but
> I do think that what's displayed is important.)

I tried it out before sending the patch and it's really not that bad.
It's says:

starting vacuum ERROR: blah
ERROR: blah
ERROR: blah
done

And then continues on.  Sure, that's not the greatest error reporting
output ever, but what do you expect from pgbench?  I think it's clear
enough what's going on there.  The messages appear in quick
succession, because it doesn't take very long to notice that the table
isn't there, so it's not like you are sitting there going "wait,
what?".

If we're going to add something, I like your second suggestion
"(ignoring this error and continuing anyway)" more than the first one.
Putting "ignoring:" before the thing you plan to ignore will be
confusing, I think.

-- 
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] pgbench -f and vacuum

[Tom Lane]

Robert Haas  writes:
> On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes  wrote:
>> But as far as what has been discussed on the central topic of this thread, I
>> think that doing the vacuum and making the failure for non-existent tables
>> be non-fatal when -f is provided would be an improvement.  Or maybe just
>> making it non-fatal at all times--if the table is needed and not present,
>> the session will fail quite soon anyway.  I don't see the other changes as
>> being improvements.  I would rather just learn to add the -n when I use -f
>> and don't have the default tables in place, than have to learn new methods
>> for saying "no really, I left -n off on purpose" when I have a custom file
>> which does use the default tables and I want them vacuumed.

> So, discussion seems to have died off here.  I think what Jeff is
> proposing here is a reasonable compromise.  Patch for that attached.

+1 as to the basic behavior, but I'm not convinced that this is
user-friendly reporting:

+   if (PQresultStatus(res) != PGRES_COMMAND_OK)
+   fprintf(stderr, "%s", PQerrorMessage(con));

I would be a bit surprised to see pgbench report an ERROR and then
continue on anyway; I might think that was a bug, even.  I am not
sure exactly what it should print instead though.  Some perhaps viable
proposals:

* don't print anything at all, just chug along.

* do something like
fprintf(stderr, "Ignoring: %s", PQerrorMessage(con));

* add something like "(Ignoring this error and continuing anyway)"
  on a line after the error message.

(I realize this takes us right back into the bikeshedding game, but
I do think that what's displayed is important.)

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] alter user/role CURRENT_USER

[Alvaro Herrera]

Kyotaro HORIGUCHI wrote:
> Thank you for completing this and very sorry not to respond these
> days.
> 
> I understood that it is committed after I noticed that rebasing
> my code failed..

You'd do well to check your email, I guess :-)

> Although after committed, I found some issues as I looked on
> it. Please forgive me to comment it now after all this time.

> 
> | =# alter role current_user rename to "PubLic";
> | ERROR:  CURRENT_USER cannot be used as a role name
> | LINE 1: alter role current_user rename to "PubLic";
> |^
> 
> The error message sounds somewhat different from the intention. I
> think the following message would be clearer.
> 
> | ERROR:  CURRENT_USER cannot be used as a role name here

Okay, changed.


> 
> The document sql-altergroup.html says
> 
> | ALTER GROUP role_specification ADD USER user_name [, ... ]
> 
> But current_user is also usable in user_name list. So the doc
> should be as following, but it would not be necessary to be fixed
> because it is an obsolete commnand..
> 
> | ALTER GROUP role_specification ADD USER role_specification [, ... ]

Yeah, EDONTCARE.

> "ALTER GROUP role_spec ADD/DROP USER role_spec" is naturally
> denied so I think no additional description is needed.

+1

> 
> sql-alterpolicy.html
> 
> "ALTER POLICY name ON table_name TO" also accepts current_user
> and so as the role to which the policy applies.

Changed.

> # As a different topic, the syntax "ALTER POLICY  ON
> #  TO " looks a bit wired, it might be better be to
> # be "ON  APPLY TO " but I shouldn't try to fix it
> # since it is a long standing syntax..

Yeah, it's a bit strange.  Not a strong opinion.  Maybe you should raise
it as a separate thread.

> 
> sql-createtablespace.html
> sql-drop-owned.html, sql-reassign-owned.html

Changed.

> ==
> sql-grant.html, sql-revoke.html,
> 
> "GRANT  TO " and "REVOKE  FROM " are
> the modern equivalents of the deprecated syntaxes "ALTER 
> ADD USER " and "ALTER  DROP USER "
> respectively. But the current parser infrastructure doesn't allow
> coexistence of the two following syntaxes but I couldn't find the
> way to their coexistence.

I decided to leave this out.  I think we should consider it as a new
patch for 9.6; these changes aren't as clear-cut as the rest of your
patch.  I didn't want to have to research the ecpg changes.

-- 
Á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] EvalPlanQual behaves oddly for FDW queries involving system columns

[Tom Lane]

Robert Haas  writes:
> Tom, you're listed as the committer for this in the CF app.  Are you
> still planning to take care of this?
> It seems that time is beginning to run short.

Yeah, I will address this (and start looking at GROUPING SETS) next week.
I'm out of town right now.

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] pgbench -f and vacuum

[Robert Haas]

On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes  wrote:
> But as far as what has been discussed on the central topic of this thread, I
> think that doing the vacuum and making the failure for non-existent tables
> be non-fatal when -f is provided would be an improvement.  Or maybe just
> making it non-fatal at all times--if the table is needed and not present,
> the session will fail quite soon anyway.  I don't see the other changes as
> being improvements.  I would rather just learn to add the -n when I use -f
> and don't have the default tables in place, than have to learn new methods
> for saying "no really, I left -n off on purpose" when I have a custom file
> which does use the default tables and I want them vacuumed.

So, discussion seems to have died off here.  I think what Jeff is
proposing here is a reasonable compromise.  Patch for that attached.

Objections?

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


pgbench-vacuum-failure-not-fatal.patch
Description: binary/octet-stream

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

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

[Robert Haas]

On Thu, Apr 16, 2015 at 2:55 AM, Etsuro Fujita
 wrote:
> Ah, you are right.  FOR NO KEY UPDATE and FOR KEY SHARE would be useful in
> the Postgres FDW if we assume the user performs those properly based on
> information about keys for a remote table.
>
> Sorry, my explanation was not correct, but I want to make it clear that the
> proposed patch also allows the FDW to change the behavior of FOR NO KEY
> UPDATE and/or FOR KEY SHARE row locking so as to match the local semantics
> exactly.
>
> BTW, I revised docs a bit.  Attached is an updated version of the patch.

Tom, you're listed as the committer for this in the CF app.  Are you
still planning to take care of this?

It seems that time is beginning to run short.

-- 
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] initdb -S and tablespaces

[Robert Haas]

On Thu, Apr 16, 2015 at 9:24 AM, Abhijit Menon-Sen  wrote:
> Here's a variation of the earlier patch that follows all links in
> PGDATA. Does this look more like what you had in mind?

I'm really confused by the additional control-file field.  It is
documented as indicating whether fsync was ever disabled while the
server was running.  But:

1. It doesn't do that.  As soon as we fsync the data directory, we
reset the flag.  That's not what "ever disabled" means to me.

2. I don't know why it's part of this patch.  Tracking whether fsync
was ever disabled could be useful forensically, but isn't related to
fsync-ing the data directory after a crash, so I dunno why we'd put
that in this patch.  Tracking whether fsync was disabled recently, as
the patch actually does, doesn't seem to be of any obvious value in
preventing corruption either.

Also, it seems awfully unfortunate to me that we're duplicating a
whole pile of code into xlog.c here.  Maybe there's no way to avoid
the code duplication, but pre_sync_fname() seems like it'd more
naturally go in fd.c than here.  I dunno where walkdir should go, but
again, not in xlog.c.

-- 
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] parallel mode and parallel contexts

[Robert Haas]

On Wed, Apr 29, 2015 at 12:23 PM, Robert Haas  wrote:
> So, I think it makes sense to split up this patch in two.  There's no
> real debate, AFAICS, about anything in the patch other than the
> heavyweight locking stuff.  So I'd like to go ahead and commit the
> rest.  That's attached here as parallel-mode-v10.patch.

Hearing no objections, done.

Still hoping for some input on the rest.

-- 
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] ERROR: unexpected data beyond EOF

[Joshua D. Drake]

On 04/30/2015 12:09 PM, Alvaro Herrera wrote:


Joshua D. Drake wrote:


I take that back, it appears this table is heavily deleted from and also
uses the lo_manage() triggers.


Well, if it's heavily deleted, then it's probably also heavily vacuumed
and from time to time empty pages at the tail are removed by vacuum.  It
might also be the case I was remembering, rather than regular TRUNCATE.

I don't think the vacuumlo stuff would have much to do with it issue; I
think it would only scan the table, then delete stuff from
pg_largeobject.  It doesn't modify the table itself (unless I'm
misremembering)

Anyway, I don't remember that we reached any useful conclusion.  Andres
suspected a PG bug, but we didn't find anything.


Well it certainly causes an outage unfortunately. Once the error occurs 
postgresql will stop accepting requests (which is correct but still).


JD


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] ERROR: unexpected data beyond EOF

[Alvaro Herrera]

Joshua D. Drake wrote:
> 
> I take that back, it appears this table is heavily deleted from and also
> uses the lo_manage() triggers.

Well, if it's heavily deleted, then it's probably also heavily vacuumed
and from time to time empty pages at the tail are removed by vacuum.  It
might also be the case I was remembering, rather than regular TRUNCATE.

I don't think the vacuumlo stuff would have much to do with it issue; I
think it would only scan the table, then delete stuff from
pg_largeobject.  It doesn't modify the table itself (unless I'm
misremembering)

Anyway, I don't remember that we reached any useful conclusion.  Andres
suspected a PG bug, but we didn't find anything.

-- 
Á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] ERROR: unexpected data beyond EOF

[Joshua D. Drake]

I take that back, it appears this table is heavily deleted from and also 
uses the lo_manage() triggers.


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] Faster setup_param_list() in plpgsql

[Pavel Stehule]

2015-04-29 9:26 GMT+02:00 Pavel Stehule :

> Hi all
>
> I am looking on this patch. I can confirm 10-15% speedup - and the idea
> behind this patch looks well.
>
> This patch
> http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us
> contains two parts
>
> a) relative large refactoring
>
> b) support for resettable fields in param_list instead total reset
>
> I believe it should be in two separate patches. Refactoring is trivial and
> there is no any possible objection.
>

I was wrong, there is relative strong dependency between these two parts,
so it should be commited as one patch

Regards

Pavel


>
> Regards
>
> Pavel
>

Re: [HACKERS] ERROR: unexpected data beyond EOF

[Joshua D. Drake]

On 04/30/2015 10:28 AM, Alvaro Herrera wrote:


Joshua D. Drake wrote:


Alright folks,

So I have this error:

postgres[21118]: [8-1] ERROR:  unexpected data beyond EOF
in block 9 of relation base/430666195/430666206

Which produces this lovely hint:

postgres[21118]: [8-2] HINT:  This has been seen to occur with buggy
kernels; consider updating your system.

However, the problem is, all relevant information we can find is Linux/NFS
based.


I have vague recollections of seeing this relatively recently on tables
that were truncated often.  Is that the case here?


Just dug into the table a bit and yes, it appears to be a transform 
table. Further questions/thoughts?


JD


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] Relation extension scalability

[Qingqing Zhou]

On Fri, Apr 17, 2015 at 11:19 AM, Qingqing Zhou
 wrote:
>
> Most commercial database employs a DMS storage model, where it manages
> object mapping and freespace itself. So different objects are sharing
> storage within several files. Surely it has historic reasons, but it
> has several advantages over current model:
> - remove fd pressure
> - remove double buffering (by introducing ADIO)
> - controlled layout and access pattern (sequential and read-ahead)
> - better quota management
> - performance potentially better
>
> Considering platforms supported and the stableness period needed, we
> shall support both current storage model and DMS model. I will stop
> here to see if this deserves further discussion.
>

Sorry, it might considered double-posting here but I am wondering have
we ever discussed this before? If we already have some conclusions on
this, could anyone share me a link?

Thanks,
Qingqing


-- 
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] Use outerPlanState() consistently in executor code

[Qingqing Zhou]

On Thu, Apr 30, 2015 at 8:02 AM, Robert Haas  wrote:
> I don't mind the MSDOS newlines, but the UTF-16le bit is inconvenient.
> UTF-8 would be much better, so I don't have to figure out how to
> convert.
>

The patch is generated via github windows tool and that's possibly
why. I regenerated it in Linux box and see attached (sending this
email in Windows and I hope no magic happens in-between).

Please let me know if that works.

Thank you,
Qingqing
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 9ff0eff..672825a 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2054,10 +2054,11 @@ ExecReScanAgg(AggState *node)
 {
ExprContext *econtext = node->ss.ps.ps_ExprContext;
int aggno;
+   PlanState   *outerPlan;

node->agg_done = false;
-
node->ss.ps.ps_TupFromTlist = false;
+   outerPlan = outerPlanState(node);

if (((Agg *) node->ss.ps.plan)->aggstrategy == AGG_HASHED)
{
@@ -2075,7 +2076,7 @@ ExecReScanAgg(AggState *node)
 * parameter changes, then we can just rescan the existing hash 
table;
 * no need to build it again.
 */
-   if (node->ss.ps.lefttree->chgParam == NULL)
+   if (outerPlan->chgParam == NULL)
{
ResetTupleHashIterator(node->hashtable, 
&node->hashiter);
return;
@@ -2133,8 +2134,8 @@ ExecReScanAgg(AggState *node)
 * if chgParam of subnode is not null then plan will be re-scanned by
 * first ExecProcNode.
 */
-   if (node->ss.ps.lefttree->chgParam == NULL)
-   ExecReScan(node->ss.ps.lefttree);
+   if (outerPlan->chgParam == NULL)
+   ExecReScan(outerPlan);
 }


diff --git a/src/backend/executor/nodeBitmapHeapscan.c 
b/src/backend/executor/nodeBitmapHeapscan.c
index 8ea8b9f..6502841 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -449,6 +449,8 @@ ExecBitmapHeapScan(BitmapHeapScanState *node)
 void
 ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 {
+   PlanState   *outerPlan;
+
/* rescan to release any page pin */
heap_rescan(node->ss.ss_currentScanDesc, NULL);

@@ -469,8 +471,9 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 * if chgParam of subnode is not null then plan will be re-scanned by
 * first ExecProcNode.
 */
-   if (node->ss.ps.lefttree->chgParam == NULL)
-   ExecReScan(node->ss.ps.lefttree);
+   outerPlan = outerPlanState(node);
+   if (outerPlan->chgParam == NULL)
+   ExecReScan(outerPlan);
 }

 /* 
diff --git a/src/backend/executor/nodeGroup.c b/src/backend/executor/nodeGroup.c
index 83d562e..b0d5442 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -280,6 +280,8 @@ ExecEndGroup(GroupState *node)
 void
 ExecReScanGroup(GroupState *node)
 {
+   PlanState   *outerPlan;
+
node->grp_done = FALSE;
node->ss.ps.ps_TupFromTlist = false;
/* must clear first tuple */
@@ -289,7 +291,7 @@ ExecReScanGroup(GroupState *node)
 * if chgParam of subnode is not null then plan will be re-scanned by
 * first ExecProcNode.
 */
-   if (node->ss.ps.lefttree &&
-   node->ss.ps.lefttree->chgParam == NULL)
-   ExecReScan(node->ss.ps.lefttree);
+   outerPlan = outerPlanState(node);
+   if (outerPlan->chgParam == NULL)
+   ExecReScan(outerPlan);
 }
diff --git a/src/backend/executor/nodeMaterial.c 
b/src/backend/executor/nodeMaterial.c
index 1158825..41859e0 100644
--- a/src/backend/executor/nodeMaterial.c
+++ b/src/backend/executor/nodeMaterial.c
@@ -317,6 +317,9 @@ ExecMaterialRestrPos(MaterialState *node)
 void
 ExecReScanMaterial(MaterialState *node)
 {
+   PlanState   *outerPlan;
+
+   outerPlan = outerPlanState(node);
ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);

if (node->eflags != 0)
@@ -339,13 +342,13 @@ ExecReScanMaterial(MaterialState *node)
 * Otherwise we can just rewind and rescan the stored output. 
The
 * state of the subnode does not change.
 */
-   if (node->ss.ps.lefttree->chgParam != NULL ||
+   if (outerPlan->chgParam != NULL ||
(node->eflags & EXEC_FLAG_REWIND) == 0)
{
tuplestore_end(node->tuplestorestate);
node->tuplestorestate = NULL;
-   if (node->ss.ps.lefttree->chgParam == NULL)
-   ExecReScan(node->ss.ps.lefttree);
+   if (outerPlan->chgParam == NULL)
+   ExecReScan(outerPlan);
node->eof_underl

Re: [HACKERS] ERROR: unexpected data beyond EOF

[Alvaro Herrera]

Joshua D. Drake wrote:
> 
> Alright folks,
> 
> So I have this error:
> 
> postgres[21118]: [8-1] ERROR:  unexpected data beyond EOF
> in block 9 of relation base/430666195/430666206
> 
> Which produces this lovely hint:
> 
> postgres[21118]: [8-2] HINT:  This has been seen to occur with buggy
> kernels; consider updating your system.
> 
> However, the problem is, all relevant information we can find is Linux/NFS
> based.

I have vague recollections of seeing this relatively recently on tables
that were truncated often.  Is that the case here?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

[HACKERS] cost_index() and path row estimate.

[Bernd Helmle]

While looking into a customer performance problem, i saw this in
costsize.c, cost_index() (9.3.6, but it looks the same in HEAD):

/* Mark the path with the correct row estimate */
if (path->path.param_info)
{
path->path.rows = path->path.param_info->ppi_rows;
/* also get the set of clauses that should be enforced by the 
scan */
allclauses = 
list_concat(list_copy(path->path.param_info->ppi_clauses),
 
baserel->baserestrictinfo);
}
else
{
path->path.rows = baserel->rows;
/* allclauses should just be the rel's restriction clauses */
allclauses = baserel->baserestrictinfo;
}

What i'm wondering is the else branch, where the baserel row estimate is
assigned to the
IndexPath. However, it occurs to me that in conjunction with a partial
index, the overall row estimate will be constrained by the row estimate
from the partial index itself, no? I see that this doesn't have an impact
on the cost estimation of the index scan itself, since cost_index() does
this later:

/* estimate number of main-table tuples fetched */
tuples_fetched = clamp_row_est(indexSelectivity * baserel->tuples);

I stumpled across this, since i see heavy misestimates in the EXPLAIN
output, where the estimated row count from the partial index vs. the real
row count heavily differs. In the customers case, there are two tables,
where one of the relation has many tuples in the JOIN condition which are
NULLs, like:

CREATE TABLE a2(id integer);
CREATE TABLE b2(id integer);

INSERT INTO a2 SELECT NULL FROM generate_series(1, 9800) AS t(id);
INSERT INTO a2 SELECT t.id FROM generate_series(1, 200) AS t(id);
INSERT INTO b2 SELECT t.id FROM generate_series(1, 200) AS t(id);

CREATE INDEX ON a2(id) WHERE id IS NOT NULL;
CREATE INDEX ON b2(id);

Here i get the following plan:

EXPLAIN ANALYZE SELECT * FROM b2 INNER JOIN a2 ON a2.id = b2.id ;

 Merge Join  (cost=10.79..12.63 rows=4 width=8) (actual time=0.084..0.291
rows=200 loops=1)
   Merge Cond: (b2.id = a2.id)
   ->  Sort  (cost=10.64..11.14 rows=200 width=4) (actual time=0.069..0.082
rows=200 loops=1)
 Sort Key: b2.id
 Sort Method: quicksort  Memory: 35kB
 ->  Seq Scan on b2  (cost=0.00..3.00 rows=200 width=4) (actual
time=0.010..0.027 rows=200 loops=1)
   ->  Index Only Scan using a2_id_idx on a2  (cost=0.14..15.14 rows=1
width=4) (actual time=0.012..0.074 rows=200 loops=1)
 Heap Fetches: 200
 Total runtime: 0.350 ms

EXPLAIN ANALYZE SELECT * FROM b2 INNER JOIN a2 ON a2.id = b2.id WHERE a2.id
IS NOT NULL;

 Merge Join  (cost=10.79..12.12 rows=1 width=8) (actual time=0.080..0.281
rows=200 loops=1)
   Merge Cond: (b2.id = a2.id)
   ->  Sort  (cost=10.64..11.14 rows=200 width=4) (actual time=0.063..0.070
rows=200 loops=1)
 Sort Key: b2.id
 Sort Method: quicksort  Memory: 35kB
 ->  Seq Scan on b2  (cost=0.00..3.00 rows=200 width=4) (actual
time=0.010..0.034 rows=200 loops=1)
   ->  Index Only Scan using a2_id_idx on a2  (cost=0.14..15.64 rows=200
width=4) (actual time=0.012..0.052 rows=200 loops=1)
 Index Cond: (id IS NOT NULL)
 Heap Fetches: 200
 Total runtime: 0.335 ms

With the partial index predicate explicitly specified the row estimate is
accurate, without the predicate the row estimate of the index scan on
a2_id_idx assumes 1.

It's very likely i miss something really important here, could someone shed
some light on this?

-- 
Thanks

Bernd


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

[HACKERS] ERROR: unexpected data beyond EOF

[Joshua D. Drake]

Alright folks,

So I have this error:

postgres[21118]: [8-1] ERROR:  unexpected data beyond EOF
in block 9 of relation base/430666195/430666206

Which produces this lovely hint:

postgres[21118]: [8-2] HINT:  This has been seen to occur with buggy 
kernels; consider updating your system.


However, the problem is, all relevant information we can find is 
Linux/NFS based.


Now it is no secret that Pg does some weird things on NFS or Virtualized 
volumes but I am not sure where to even start with this.


The system is:

FreeBSD 10
ZFS
iSCSI
RAID 50 (don't start, I didn't spec it).
fsync on, full_page_writes on

The restart of PostgreSQL makes the error go away and things progress 
normally. We don't experience further errors etc...


Any thoughts on this?

JD




--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] feature freeze and beta schedule

[Robert Haas]

On Thu, Apr 30, 2015 at 12:52 PM, Peter Eisentraut  wrote:
> On 4/30/15 12:01 PM, Robert Haas wrote:
>> So generally we have stamped in late April or early May and released
>> in September, but last year we didn't release until December.  I
>> assume that if we stamp beta1 in June instead of May, that's going to
>> somewhat delay the final release as well, but I'm not sure how much.
>
> The idea when that schedule was cobbled together in the dying minutes of
> the last developer meeting ;-) was to have a shorter beta and still
> shoot for a September release.

I think that could be doable if we can keep focus, but that's easier
said than done.

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


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

Re: [HACKERS] pg_upgrade: quote directory names in delete_old_cluster script

[Bruce Momjian]

On Wed, Apr 29, 2015 at 11:59:26PM -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
> > I have developed the attached patch to use platform-specific quoting of
> > path names.
> 
> Part of me wonders about initdb's existing DIR_SEP and QUOTE_PATH
> definitions ... seems messy to reinvent these things all over again.

I don't think we can reuse QUOTE_PATH as it is used in initdb for
displaying potential ways of starting the server, and it assumes Unix
doesn't need quoting, while Windows usually does.  For an actual script,
we always want to use quoting.

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

  + Everyone has their own god. +


-- 
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] feature freeze and beta schedule

[Peter Eisentraut]

On 4/30/15 12:01 PM, Robert Haas wrote:
> So generally we have stamped in late April or early May and released
> in September, but last year we didn't release until December.  I
> assume that if we stamp beta1 in June instead of May, that's going to
> somewhat delay the final release as well, but I'm not sure how much.

The idea when that schedule was cobbled together in the dying minutes of
the last developer meeting ;-) was to have a shorter beta and still
shoot for a September release.



-- 
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] BRIN range operator class

[Alvaro Herrera]

Robert Haas wrote:
> On Mon, Apr 6, 2015 at 5:17 PM, Alvaro Herrera  
> wrote:
> > Thanks for the updated patch; I will at it as soon as time allows.  (Not
> > really all that soon, regrettably.)
> >
> > Judging from a quick look, I think patches 1 and 5 can be committed
> > quickly; they imply no changes to other parts of BRIN.  (Not sure why 1
> > and 5 are separate.  Any reason for this?)  Also patch 2.
> >
> > Patch 4 looks like a simple bugfix (or maybe a generalization) of BRIN
> > framework code; should also be committable right away.  Needs a closer
> > look of course.
> 
> Is this still pending?

Yeah.

-- 
Á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] Proposal : REINDEX xxx VERBOSE

[Robert Haas]

On Thu, Apr 30, 2015 at 9:15 AM, Sawada Masahiko  wrote:
> On Thu, Apr 30, 2015 at 8:39 PM, Robert Haas  wrote:
>> On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko  
>> wrote:
>>> Attached v10 patch is latest version patch.
>>> The syntax is,
>>> REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
>>>
>>> That is, WITH clause is optional.
>>
>> I thought we agreed on moving this earlier in the command:
>>
>> http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
>>
>
> Oh, I see.
> Attached patch is modified syntax as
> REINDEX [VERBOSE] { INDEX | ... } name
>
> Thought?

I thought what we agreed on was:

REINDEX (flexible options) { INDEX | ... } name

The argument wasn't about whether to use flexible options, but where
in the command to put them.

-- 
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] Proposal : REINDEX xxx VERBOSE

[Fabrízio de Royes Mello]

On Thu, Apr 30, 2015 at 10:15 AM, Sawada Masahiko 
wrote:
>
> On Thu, Apr 30, 2015 at 8:39 PM, Robert Haas 
wrote:
> > On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko 
wrote:
> >> Attached v10 patch is latest version patch.
> >> The syntax is,
> >> REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
> >>
> >> That is, WITH clause is optional.
> >
> > I thought we agreed on moving this earlier in the command:
> >
> > http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
> >
>
> Oh, I see.
> Attached patch is modified syntax as
> REINDEX [VERBOSE] { INDEX | ... } name
>
> Thought?
>

Looks good to me.

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] feature freeze and beta schedule

[Robert Haas]

On Thu, Apr 30, 2015 at 8:39 AM, Peter Eisentraut  wrote:
> The schedule
> 
> calls for beta in June.  In light of that, the core team has agreed to
> call for
>
> feature freeze on May 15
>
> That means that all patches that add or change features should be
> committed by then.
>
> If you have spare cycles, there are a number of relevant patches still
> open in the commit fest.

What do we think this means in terms of the schedule for the final release?

Looking at the history:

9.4beta1 was stamped on May 11th; 9.4 was released December 18th.
9.3beta1 was stamped on May 6th; 9.3 was released September 9th.
9.2beta1 was stamped on May 10th; 9.2 was released September 10th.
9.1beta1 was stamped on April 27th; 9.1 was released September 12th.
9.0beta1 was stamped on April 30th; 9.0 was released September 20th.

So generally we have stamped in late April or early May and released
in September, but last year we didn't release until December.  I
assume that if we stamp beta1 in June instead of May, that's going to
somewhat delay the final release as well, but I'm not sure how much.

Looking at the CF page, we're doing better than I thought: we're down
from 113 patches to just 50.  That's good, but it's still an awful lot
to handle in the next two weeks.

-- 
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] Reducing tuple overhead

[Alvaro Herrera]

Robert Haas wrote:
> On Mon, Apr 27, 2015 at 5:01 PM, Jim Nasby  wrote:
> > The problem with just having the value is that if *anything* changes between
> > how you evaluated the value when you created the index tuple and when you
> > evaluate it a second time you'll corrupt your index. This is actually an
> > incredibly easy problem to have; witness how we allowed indexing
> > timestamptz::date until very recently. That was clearly broken, but because
> > we never attempted to re-run the index expression to do vacuuming at least
> > we never corrupted the index itself.
> 
> True.  But I guess what I don't understand is: how big a deal is this,
> really?  The "uncorrupted" index can still return wrong answers to
> queries.  The fact that you won't end up with index entries pointing
> to completely unrelated tuples is nice, but if index scans are missing
> tuples that they should see, aren't you still pretty hosed?

As I recall, there's a desire not to run expressions when vacuuming,
because to run them means getting a snapshot, in case any functions look
at database state; and you can't get a snapshot while vacuuming because
that means the vacuum gets visible to concurrent processes; in
particular they keep all processes' xmin from moving forward.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] pg_upgrade: quote directory names in delete_old_cluster script

[Alvaro Herrera]

Bruce Momjian wrote:

> I have developed the attached patch to use platform-specific quoting of
> path names.

Part of me wonders about initdb's existing DIR_SEP and QUOTE_PATH
definitions ... seems messy to reinvent these things all over again.

-- 
Á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] collations in shared catalogs?

[Tom Lane]

Bruce Momjian  writes:
 On 2015-02-25 12:08:32 -0500, Tom Lane wrote:
> The most obvious fix is to change "provider" to a NAME column.

> Where are we on this?

Not done yet, but we should make a point of making that fix before 9.5.
Please add it to the open items page for 9.5.

I am not sure there's anything useful to be done about this in the back
branches.

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] Use outerPlanState() consistently in executor code

[Robert Haas]

On Thu, Apr 30, 2015 at 9:02 AM, Bruce Momjian  wrote:
> On Thu, Apr 30, 2015 at 08:46:55AM -0400, Robert Haas wrote:
>> On Wed, Apr 15, 2015 at 3:38 PM, Qingqing Zhou
>>  wrote:
>> > In executor context, outerPlanState(node) is the same as
>> > node->ss.ps.lefttree. We follow this in most places except a few. This
>> > patch clean up the outliers and might save us a few instructions by
>> > removing indirection.
>> >
>> > Most of changes are trivial. Except I take out an outerPlan nullable
>> > check in grouping iterator - as a it surely has a left child.
>>
>> I don't see any particular reason not to do this.
>>
>> The patch is weird, though.  If I open it with "less", I get binary
>> garbage.  My Mac's TextEdit app opens it OK though.
>
> The patch is encoded as utf-16le, and has MSDOS newlines, ^M.

I don't mind the MSDOS newlines, but the UTF-16le bit is inconvenient.
UTF-8 would be much better, so I don't have to figure out how to
convert.

-- 
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] Precedence of NOT LIKE, NOT BETWEEN, etc

[Tom Lane]

Bruce Momjian  writes:
> On Wed, Apr  8, 2015 at 01:14:38PM -0400, Tom Lane wrote:
>> Greg Stark  writes:
>>> On Tue, Feb 24, 2015 at 5:04 PM, Tom Lane  wrote:
 Also, it strikes me that we could significantly reduce, maybe even fully
 eliminate, the funny behaviors around the existing base_yylex()
 substitutions if we made them use the same idea, ie replace the leading
 token with something special but keep the second token's separate
 identity.  Unless somebody sees a hole in this idea, I'll probably go
 do that and then come back to the precedence issues.

>>> IIRC that's exactly what the earlier patch for this did.

>> Right, see d809fd0008a2e26de463f47b7aba0365264078f3

> Where are we on this?

It's done as far as seemed reasonable to push it.

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] collations in shared catalogs?

[Bruce Momjian]

On Wed, Feb 25, 2015 at 10:19:45PM +0100, Andres Freund wrote:
> On 2015-02-25 15:59:55 -0500, Tom Lane wrote:
> > Andres Freund  writes:
> > > On 2015-02-25 12:08:32 -0500, Tom Lane wrote:
> > >> The most obvious fix is to change "provider" to a NAME column.
> > 
> > > Yea. I'm not sure why that wasn't done initially. I can't really see the
> > > length be an issue. How about we add an error check enforcing ascii,
> > > that'll work in the back branches?
> > 
> > Nope, that won't help much at all.  C vs en_US for instance is different
> > sort orders even with all-ASCII data.
> 
> Ick, yes. The restriction to a charset that's encodable in all server
> encodings should be there additionally, but it's not sufficient :(
> 
> > Basically you're screwed if you've got different collations in different
> > databases and you put anything into pg_shseclabel ...
> 
> Hrmpf.

Where are we on this?

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

  + Everyone has their own god. +


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

Re: [HACKERS] Turning recovery.conf into GUCs

[Michael Paquier]

On Sat, Feb 21, 2015 at 6:45 AM, Peter Eisentraut  wrote:
> On 2/19/15 4:33 PM, Josh Berkus wrote:
>> On 02/19/2015 12:23 PM, Peter Eisentraut wrote:
>>> On 1/6/15 4:22 PM, Peter Eisentraut wrote:
 That said, there is a much simpler way to achieve that specific
 functionality: Expose all the recovery settings as fake read-only GUC
 variables.  See attached patch for an example.
>>>
>>> The commit fest app has this as the patch of record.  I don't think
>>> there is a real updated patch under consideration here.  This item
>>> should probably not be in the commit fest at all.
>>
>> What happened to the original patch?  Regardless of what we do, keeping
>> recovery.conf the way it is can't be what we go forward with.
>
> There was disagreement on many of the details, and no subsequent new
> proposal.

Patch has been marked as "Returned with feedback".
-- 
Michael


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

Re: [HACKERS] Moving on to close the current CF 2015-02

[Michael Paquier]

On Wed, Apr 29, 2015 at 1:10 AM, Magnus Hagander  wrote:
> On Fri, Apr 17, 2015 at 4:57 PM, Magnus Hagander 
> wrote:
>>
>> On Fri, Apr 17, 2015 at 9:23 AM, Michael Paquier
>>  wrote:
>>>
>>> On Fri, Apr 17, 2015 at 4:22 PM, Michael Paquier wrote:
>>> > @Magnus: having the possibility to mark a patch as "returned with
>>> > feedback" without bumping it to the next CF automatically would be
>>> > cool to being moving on.
>>> Meh. "cool to have to help moving on".
>>>
>>
>> Yeah, it's at the top of my list of priorities once I get some time to
>> spend on community stuff. Hopefully I can get around to it next week. There
>> is a small chance I can do it before then, but it is indeed small...
>
>
> My apologies for that being delayed even longe rthan that. I've finally
> pushed the changes that:
>
> * Renames the current "returned with feedback" to "moved to next cf"
> * Adds a new status, "returned with feedback", that is the same as
> "rejected" in everything except the label (meaning it closes the patch out,
> but does *not* move it to the next CF).
>
> This was at least my understanding of the consensus :)

Thanks a lot for this! This looks neat to me.
-- 
Michael


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

Re: [HACKERS] contrib/fuzzystrmatch/dmetaphone.c license

[Bruce Momjian]

On Wed, Feb 25, 2015 at 08:36:49PM -0500, Andrew Dunstan wrote:
> >>>I doubt we want to rip it out without some suitable
> >>>replacement -- do we?
> >>>
> >>>
> >>
> >>That's more than 10 years ago. I remember creating this for my then work
> >>at the North Carolina State Highway Patrol and sending it to Joe, but
> >>that's about the extent of my recollection.
> >>
> >>If the Artistic License isn't acceptable. I guess we'd have to try to
> >>get the code relicensed, or reimplement the function ourselves. There
> >>are numerous implementations out there we could copy from or use as a
> >>basis for reimplementation, including several licensed under the Apache
> >>2.0 license - is that compatible with ours?
> >
> >Perhaps a company large enough to have in-house counsel
> >(EnterpriseDB?) could get a quick legal opinion on the license
> >before we start pursuing other things? Perhaps this is just a
> >non-issue...
> 
> 
> The first para above was written by Dave Page, who works for EDB 

Where are we on this?

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

  + Everyone has their own god. +


-- 
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] Reducing tuple overhead

[Robert Haas]

On Thu, Apr 30, 2015 at 9:46 AM, Amit Kapila  wrote:
> As the index expression contain table columns and all the functions
> or operators used in expression must be IMMUTABLE, won't that
> guarantee to avoid such a situation?

The concern is that they might be labeled as immutable but not
actually behave that way.

The other, related problem is that the ordering operator might start
to return different results than it did at index creation time.  For
example, consider a btree index built on a text column.  Now consider
'yum update'.  glibc gets updated, collation ordering of various
strings change, and now you've got tuples that are in the "wrong
place" in the index, because when the index was built, we thought A <
B, but now we think B < A.  You would think the glibc maintainers
might avoid such changes in minor releases, or that the Red Hat guys
would avoid packaging and shipping those changes in minor releases,
but you'd be wrong.  We've seen cases where the master and the standby
were both running RHEL X.Y (same X.Y on both servers) but they didn't
agree on the collation definitions, so queries that worked on the
master failed on the slave.

> If not, then I think for such indexes we might need to search
> for tupleoffset in the entire index and mark it as Delete or may be
> for such indexes follow the current mechanism.

I think that *is* the current mechanism.

> I think it will still
> give us lot of benefit in more common cases.

It's hard to figure out exactly what can work here.  Aside from
correctness issues, the biggest problem with refinding the index
tuples one-by-one and killing them is that it may suck for bulk
operations.  When you delete one tuple from the table, refinding the
index tuples and killing them immediately is probably the smartest
thing you can do.  If you postpone it, somebody may be forced to split
the page because it's full, whereas if you do it right away, the next
guy who wants to put a tuple on that page may be able to make it fit.
That's a big win.

But if you want to delete 10 million tuples, doing an index scan for
each one may induce a tremendous amount of random I/O on the index
pages, possibly visiting and dirtying the same pages more than once.
Scanning the whole index for tuples to kill avoids that.

-- 
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] Reducing tuple overhead

[Amit Kapila]

On Thu, Apr 30, 2015 at 5:03 PM, Robert Haas  wrote:
>
> On Thu, Apr 30, 2015 at 12:31 AM, Amit Kapila 
wrote:
> > I think I am missing something here, but when this second
> > evaluation is needed.  Basically what I understand from index
> > insertion is that it evaluates the value to be inserted in index
> > before calling nbtree module and then nbtree just inserts the
> > value/tuple passed to it.
>
> Sure, but what happens if it doesn't evaluate to the same value?
>
> Consider a tuple where a = 1 and a function f(a).  You insert the
> tuple, evaluate f(a), and get 17.  So you insert an index tuple into
> the btree with a value of 17, pointing at the tuple.  Now you delete
> the tuple, evaluate f(a) again, and this time you get 42.

As the index expression contain table columns and all the functions
or operators used in expression must be IMMUTABLE, won't that
guarantee to avoid such a situation?

If not, then I think for such indexes we might need to search
for tupleoffset in the entire index and mark it as Delete or may be
for such indexes follow the current mechanism.  I think it will still
give us lot of benefit in more common cases.


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

Re: [HACKERS] Loss of some parts of the function definition

[Pavel Stehule]

2015-04-30 15:34 GMT+02:00 Sergey Grinko :

> I agree that it is better to show what really works.
> I propose to allow additional option through a source code which is made
> on the basis of a compilation of metadata.
> This will solve the problem.
>

You can to teach PostgreSQL function to use precision and derived types -
it is not plpgsql issue only - it is related to all PL.

There was some proposals in this area. Currently it is much better
situation than year ago, because plpgsql use binary cast instead IO cast
now.

Regards

Pavel Stehule


>
> 2015-04-30 16:19 GMT+03:00 Pavel Stehule :
>
>>
>>
>> 2015-04-30 15:08 GMT+02:00 Sergey Grinko :
>>
>>> That's what I have to do now.
>>> But there is some problem.
>>> When you try to build the update script I get to Git code is always
>>> different from what I see in the database.
>>> It is not right.
>>> MSSQL Server, Oracle, ... always saving of the full text DDL.
>>> I do not understand why PostgreSQL believe that part of the source
>>> function must be removed !?
>>>
>>
>> I can understand to problem, but it doesn't help to you. Postgres
>> displays the code, that is really used. So we can speak what is more wrong
>> - displaying original but not used code, or displaying really used code.
>>
>> I am thinking so current solution is better - any other solution mean 2x
>> stored data, that can be partially inconsistent.
>>
>> It cannot be comparable with Oracle - because it is different technology.
>>
>>
>>>
>>> 2015-04-30 15:59 GMT+03:00 Pavel Stehule :
>>>


 2015-04-30 14:52 GMT+02:00 Sergey Grinko :

> Yes, I understand that.
> So I ask to implement saving of the full text DDL.
> This will allow developers to be able to save a meaning at the level
> of the source code.
> I ask to make sure that the function pg_get_function_def () returns
> previously stored full text DDL, instead of generating input and output
> parameters based on metadata.
>

 I don't see a sense of this - usually much better is storing code to
 files and using GIT and other.

 Surely, you can safe code to any custom table.

 Regards

 Pavel


>
> 2015-04-30 15:46 GMT+03:00 Pavel Stehule :
>
>> Hi
>>
>> 2015-04-30 13:44 GMT+02:00 Sergey Grinko :
>>
>>> Hi,
>>>
>>> Dear developers, I have a request to you.
>>>
>>> Now create a script in the application of its function parameters
>>> and return values can be declared using %TYPE.
>>> However, when you save the script is stored inside the server only
>>> what is considered his body. Thus, we obtain:
>>> 1) loss of the custom formatting.
>>> 2) loss of communication parameters and return types with these
>>> types of fields to create the function.
>>> 3) multidimensional arrays are transformed into one-dimensional:
>>> [][] -> []
>>> 4) loss of data accuracy: numeric(n,m) -> numeric
>>>
>>> Please - how to save and restore the entire text of the definition
>>> to CREATE END; unchanged.
>>>
>>
>> I am afraid, it is not possible
>>
>> Postgres doesn't distinguish between multidimensional and one
>> dimensional arrays - multidimensional is just syntax suger, same is
>> function arguments - Postgres doesn't store precision for parameters.
>> type%TYPE is translated to target type outside plpgsql function. These
>> informations are not saved, so you cannot to take it from PostgreSQL
>>
>> Regards
>>
>> Pavel Stehule
>>
>>
>>
>>
>>
>>
>>>
>>>
>>> --
>>> Yours faithfully, Sergey Grinko
>>> Email: sergey.gri...@gmail.com
>>>
>>
>>
>
>
> --
> Yours faithfully, Sergey Grinko
> Email: sergey.gri...@gmail.com
>


>>>
>>>
>>> --
>>> Yours faithfully, Sergey Grinko
>>> Email: sergey.gri...@gmail.com
>>>
>>
>>
>
>
> --
> Yours faithfully, Sergey Grinko
> Email: sergey.gri...@gmail.com
>

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Robert Haas]

On Thu, Apr 30, 2015 at 9:16 AM, Kouhei Kaigai  wrote:
> It seems to me the code block for T_ForeignScan and T_CustomScan in
> setrefs.c are a bit large. It may be better to have a separate
> function like T_IndexOnlyScan.
> How about your opinion?

Either way is OK with me.  Please do as you think best.

>> +* Sanity check. Pseudo scan tuple-descriptor shall be constructed
>> +* based on the fdw_ps_tlist, excluding resjunk=true, so we need to
>> +* ensure all valid TLEs have to locate prior to junk ones.
>>
>> Is the goal here to make attribute numbers match up?  If so, between
>> where and where?  If not, please explain further.
>>
> No, its purpose is to reduce unnecessary projection.
>
> The *_ps_tlist is not only used to construct tuple-descriptor of
> Foreign/CustomScan with scanrelid==0, but also used to resolve var-
> nodes with varno==INDEX_VAR in EXPLAIN command.
>
> For example,
>   SELECT t1.y, t2.b FROM t1, t2 WHERE t1.x = t2.a;
>
> If "t1.x = t2.a" is executable on external computing resource (like
> remote RDBMS or GPU device, etc), both of t1.x and t2.a don't need
> to appear on the targetlist of joinrel.
> In this case, the best *_ps_tlist consists of two var-nodes of t1.x
> and t2.a because it fits tuple-descriptor of result tuple slot, thus
> it can skip per-tuple projection.
>
> On the other hands, we may want to print out expression clause that
> shall be executed on the external resource; "t1.x = t2.a" in this
> case. If FDW/CSP keeps this clause in expression form, its var-nodes
> shall be rewritten to a pair of INDEX_VAR and resno on *_ps_tlist.
> So, deparse_expression() needs to be capable to find out "t1.x" and
> "t2.a" on the *_ps_tlist. However, it does not make sense to include
> these variables on the scan tuple-descriptor.
>
> ExecInitForeignScan() and ExecInitCustomScan() makes its scan tuple-
> descriptor using ExecCleanTypeFromTL(), not ExecTypeFromTL(), to omit
> these unreferenced variables on the *_ps_tlist. All the var-nodes with
> INDEX_VAR shall be identified by offset from head of the list, we cannot
> allow any target-entry with resjunk=false after ones with resjunk=true,
> to keep the expected varattno.
>
> This sanity checks ensures no target-entry with resjunk=false after
> the resjunk=true. It helps to distinct attributes to be included in
> the result tuple from the ones for just reference in EXPLAIN.
>
> Did my explain above introduced the reason of this sanity check well?

Yeah, I think so.  So what we want to do in this comment is summarize
all of that briefly.  Maybe something like this:

"Sanity check.  There may be resjunk entries in fdw_ps_tlist that are
included only to help EXPLAIN deparse plans properly.  We require that
these are at the end, so that when the executor builds the scan
descriptor based on the non-junk entries, it gets the attribute
numbers correct."

>> +   if (splan->scan.scanrelid == 0)
>> +   {
>> ...
>> +   }
>> splan->scan.scanrelid += rtoffset;
>>
>> Does this need an "else"?  It seems surprising that you would offset
>> scanrelid even if it's starting out as zero.
>>
>> (Note that there are two instances of this pattern.)
>>
> 'break' was put on the tail of if-block, however, it may lead potential
> bugs in the future. I'll use if-else manner as usual.

Ah, OK, I missed that.  Yeah, that's probably a good change.

I assume you realize you did not attach an updated patch?

-- 
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] Precedence of NOT LIKE, NOT BETWEEN, etc

[Bruce Momjian]

On Wed, Apr  8, 2015 at 01:14:38PM -0400, Tom Lane wrote:
> Greg Stark  writes:
> > On Tue, Feb 24, 2015 at 5:04 PM, Tom Lane  wrote:
> >> Also, it strikes me that we could significantly reduce, maybe even fully
> >> eliminate, the funny behaviors around the existing base_yylex()
> >> substitutions if we made them use the same idea, ie replace the leading
> >> token with something special but keep the second token's separate
> >> identity.  Unless somebody sees a hole in this idea, I'll probably go
> >> do that and then come back to the precedence issues.
> 
> > IIRC that's exactly what the earlier patch for this did.
> 
> Right, see d809fd0008a2e26de463f47b7aba0365264078f3

Where are we on this?

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

  + Everyone has their own god. +


-- 
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] Loss of some parts of the function definition

[Sergey Grinko]

I agree that it is better to show what really works.
I propose to allow additional option through a source code which is made on
the basis of a compilation of metadata.
This will solve the problem.

2015-04-30 16:19 GMT+03:00 Pavel Stehule :

>
>
> 2015-04-30 15:08 GMT+02:00 Sergey Grinko :
>
>> That's what I have to do now.
>> But there is some problem.
>> When you try to build the update script I get to Git code is always
>> different from what I see in the database.
>> It is not right.
>> MSSQL Server, Oracle, ... always saving of the full text DDL.
>> I do not understand why PostgreSQL believe that part of the source
>> function must be removed !?
>>
>
> I can understand to problem, but it doesn't help to you. Postgres displays
> the code, that is really used. So we can speak what is more wrong -
> displaying original but not used code, or displaying really used code.
>
> I am thinking so current solution is better - any other solution mean 2x
> stored data, that can be partially inconsistent.
>
> It cannot be comparable with Oracle - because it is different technology.
>
>
>>
>> 2015-04-30 15:59 GMT+03:00 Pavel Stehule :
>>
>>>
>>>
>>> 2015-04-30 14:52 GMT+02:00 Sergey Grinko :
>>>
 Yes, I understand that.
 So I ask to implement saving of the full text DDL.
 This will allow developers to be able to save a meaning at the level of
 the source code.
 I ask to make sure that the function pg_get_function_def () returns
 previously stored full text DDL, instead of generating input and output
 parameters based on metadata.

>>>
>>> I don't see a sense of this - usually much better is storing code to
>>> files and using GIT and other.
>>>
>>> Surely, you can safe code to any custom table.
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>>

 2015-04-30 15:46 GMT+03:00 Pavel Stehule :

> Hi
>
> 2015-04-30 13:44 GMT+02:00 Sergey Grinko :
>
>> Hi,
>>
>> Dear developers, I have a request to you.
>>
>> Now create a script in the application of its function parameters and
>> return values can be declared using %TYPE.
>> However, when you save the script is stored inside the server only
>> what is considered his body. Thus, we obtain:
>> 1) loss of the custom formatting.
>> 2) loss of communication parameters and return types with these types
>> of fields to create the function.
>> 3) multidimensional arrays are transformed into one-dimensional: [][]
>> -> []
>> 4) loss of data accuracy: numeric(n,m) -> numeric
>>
>> Please - how to save and restore the entire text of the definition to
>> CREATE END; unchanged.
>>
>
> I am afraid, it is not possible
>
> Postgres doesn't distinguish between multidimensional and one
> dimensional arrays - multidimensional is just syntax suger, same is
> function arguments - Postgres doesn't store precision for parameters.
> type%TYPE is translated to target type outside plpgsql function. These
> informations are not saved, so you cannot to take it from PostgreSQL
>
> Regards
>
> Pavel Stehule
>
>
>
>
>
>
>>
>>
>> --
>> Yours faithfully, Sergey Grinko
>> Email: sergey.gri...@gmail.com
>>
>
>


 --
 Yours faithfully, Sergey Grinko
 Email: sergey.gri...@gmail.com

>>>
>>>
>>
>>
>> --
>> Yours faithfully, Sergey Grinko
>> Email: sergey.gri...@gmail.com
>>
>
>


-- 
Yours faithfully, Sergey Grinko
Email: sergey.gri...@gmail.com

Re: [HACKERS] configure can't detect proper pthread flags

[Max Filippov]

On Thu, Apr 30, 2015 at 3:51 PM, Robert Haas  wrote:
> On Wed, Apr 8, 2015 at 2:31 AM, Max Filippov  wrote:
>> On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov  wrote:
>>> On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov  wrote:
 Ok, one more attempt: maybe instead of checking that stderr is empty
 we could check that stderr has changed in the presence of the option
 that we test?
>>>
>>> The patch:
>>> http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvb...@gmail.com
>>
>> Ping?
>> Are there any issues with that approach and the patch?
>
> I think the thing to do is add your patch here so that it doesn't get
> forgotten about:
>
> https://commitfest.postgresql.org/action/commitfest_view/open

Done: https://commitfest.postgresql.org/5/232/

-- 
Thanks.
-- Max


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

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Kouhei Kaigai]

> On Sun, Apr 26, 2015 at 10:00 PM, Kouhei Kaigai  wrote:
> > The attached patch v13 is revised one according to the suggestion
> > by Robert.
> 
> Thanks.
> 
> The last hunk in foreign.c is a useless whitespace change.
>
Sorry, my oversight.

> +   /* actually, not shift members */
> 
> Change to: "shift of 0 is the same as copying"
> 
> But actually, do we really need all of this?  I think you could reduce
> the size of this function to three lines of code if you just did this:
> 
> x = -1;
> while ((x = bms_next_member(inputset, x)) >= 0)
> outputset = bms_add_member(inputset, x + shift);
> 
> It might be very slightly slower, but I think it would be worth it to
> reduce the amount of code needed.
>
OK, I reverted the bms_shift_members().

It seems to me the code block for T_ForeignScan and T_CustomScan in
setrefs.c are a bit large. It may be better to have a separate
function like T_IndexOnlyScan.
How about your opinion?

> +* 5. Consider paths added by FDW, in case when both of outer and
> +* inner relations are managed by the same driver.
> 
> Change to: "If both inner and outer relations are managed by the same
> FDW, give it a chance to push down joins."
>
OK,

> +* 6. At the last, consider paths added by extension, in addition to 
> the
> +* built-in paths.
> 
> Change to: "Finally, give extensions a chance to manipulate the path list."
>
OK,

> +* Fetch relation-id, if this foreign-scan node actuall scans on
> +* a particular real relation. Elsewhere, InvalidOid shall be
> +* informed to the FDW driver.
> 
> Change to: "If we're scanning a base relation, look up the OID.  (We
> can skip this if scanning a join relation.)"
>
OK,

> +* Sanity check. Pseudo scan tuple-descriptor shall be constructed
> +* based on the fdw_ps_tlist, excluding resjunk=true, so we need to
> +* ensure all valid TLEs have to locate prior to junk ones.
> 
> Is the goal here to make attribute numbers match up?  If so, between
> where and where?  If not, please explain further.
>
No, its purpose is to reduce unnecessary projection.

The *_ps_tlist is not only used to construct tuple-descriptor of
Foreign/CustomScan with scanrelid==0, but also used to resolve var-
nodes with varno==INDEX_VAR in EXPLAIN command.

For example,
  SELECT t1.y, t2.b FROM t1, t2 WHERE t1.x = t2.a;

If "t1.x = t2.a" is executable on external computing resource (like
remote RDBMS or GPU device, etc), both of t1.x and t2.a don't need
to appear on the targetlist of joinrel.
In this case, the best *_ps_tlist consists of two var-nodes of t1.x
and t2.a because it fits tuple-descriptor of result tuple slot, thus
it can skip per-tuple projection.

On the other hands, we may want to print out expression clause that
shall be executed on the external resource; "t1.x = t2.a" in this
case. If FDW/CSP keeps this clause in expression form, its var-nodes
shall be rewritten to a pair of INDEX_VAR and resno on *_ps_tlist.
So, deparse_expression() needs to be capable to find out "t1.x" and
"t2.a" on the *_ps_tlist. However, it does not make sense to include
these variables on the scan tuple-descriptor.

ExecInitForeignScan() and ExecInitCustomScan() makes its scan tuple-
descriptor using ExecCleanTypeFromTL(), not ExecTypeFromTL(), to omit
these unreferenced variables on the *_ps_tlist. All the var-nodes with
INDEX_VAR shall be identified by offset from head of the list, we cannot
allow any target-entry with resjunk=false after ones with resjunk=true,
to keep the expected varattno.

This sanity checks ensures no target-entry with resjunk=false after
the resjunk=true. It helps to distinct attributes to be included in
the result tuple from the ones for just reference in EXPLAIN.

Did my explain above introduced the reason of this sanity check well?


> +   if (splan->scan.scanrelid == 0)
> +   {
> ...
> +   }
> splan->scan.scanrelid += rtoffset;
> 
> Does this need an "else"?  It seems surprising that you would offset
> scanrelid even if it's starting out as zero.
> 
> (Note that there are two instances of this pattern.)
>
'break' was put on the tail of if-block, however, it may lead potential
bugs in the future. I'll use if-else manner as usual.

> + * 'found' : indicates whether RelOptInfo is actually constructed.
> + * true, if it was already built and on the cache.
> 
> Leftover hunk.  Revert this.
>
Fixed,

> +typedef void (*GetForeignJoinPaths_function ) (PlannerInfo *root,
> 
> Whitespace is wrong, still.
>
Fixed,

> + * An optional fdw_ps_tlist is used to map a reference to an attribute of
> + * underlying relation(s) on a pair of INDEX_VAR and alternative varattno.
> 
> on -> onto
>
OK,

> + * It looks like a scan on pseudo relation that is usually result of
> + * relations join on remote data source, and FDW driver is responsible to
> + * set expec

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Thu, Apr 30, 2015 at 8:39 PM, Robert Haas  wrote:
> On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko  
> wrote:
>> Attached v10 patch is latest version patch.
>> The syntax is,
>> REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
>>
>> That is, WITH clause is optional.
>
> I thought we agreed on moving this earlier in the command:
>
> http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
>

Oh, I see.
Attached patch is modified syntax as
REINDEX [VERBOSE] { INDEX | ... } name

Thought?

Regards,

---
Sawada Masahiko


000_reindex_verbose_v11.patch
Description: Binary data

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

Re: [HACKERS] Re: [BUGS] BUG #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)

[Michael Paquier]

On Thu, Apr 30, 2015 at 9:53 PM, Robert Haas  wrote:
> On Sat, Mar 21, 2015 at 9:00 AM, Michael Paquier
>  wrote:
>> On Fri, Mar 20, 2015 at 9:48 PM, Bruce Momjian  wrote:
>>> On Tue, Oct 28, 2014 at 07:02:41AM +, krystian.bi...@gmail.com wrote:
 The following bug has been logged on the website:

 Bug reference:  11805
 Logged by:  Krystian Bigaj
 Email address:  krystian.bi...@gmail.com
 PostgreSQL version: 9.3.5
 Operating system:   Windows 7 Pro x64
 Description:

 pg_ctl on Windows during service start/shutdown should notify service
 manager about it's status by increment dwCheckPoint and call to
 SetServiceStatus/pgwin32_SetServiceStatus.

 However during shutdown there is a missing call to SetServiceStatus.
 See src\bin\pg_ctl\pg_ctl.c:
>>>
>>> [ thread moved to hackers ]
>>>
>>> Can a Windows person look into this issue?
>>>
>>>
>>> http://www.postgresql.org/message-id/20141028070241.2593.58...@wrigleys.postgresql.org
>>>
>>> The thread includes a patch.  I need a second person to verify its
>>> validity.  Thanks.
>>
>> FWIW, it looks sane to me to do so, ServiceMain declaration is in
>> charge to start the service, and to wait for the postmaster to stop,
>> and indeed process may increment dwcheckpoint in -w mode, and it
>> expects for process to wait for 12 times but this promise is broken.
>> The extra calls to SetServiceStatus are also welcome to let the SCM
>> know the current status in more details.
>
> So, what's next here?

I guess that a committer opinion would be welcome. IMO the current
behavior is a bug.
-- 
Michael


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

Re: [HACKERS] Use outerPlanState() consistently in executor code

[Bruce Momjian]

On Thu, Apr 30, 2015 at 08:46:55AM -0400, Robert Haas wrote:
> On Wed, Apr 15, 2015 at 3:38 PM, Qingqing Zhou
>  wrote:
> > In executor context, outerPlanState(node) is the same as
> > node->ss.ps.lefttree. We follow this in most places except a few. This
> > patch clean up the outliers and might save us a few instructions by
> > removing indirection.
> >
> > Most of changes are trivial. Except I take out an outerPlan nullable
> > check in grouping iterator - as a it surely has a left child.
> 
> I don't see any particular reason not to do this.
> 
> The patch is weird, though.  If I open it with "less", I get binary
> garbage.  My Mac's TextEdit app opens it OK though.

The patch is encoded as utf-16le, and has MSDOS newlines, ^M.

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

  + Everyone has their own god. +


-- 
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 #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)

[Robert Haas]

On Sat, Mar 21, 2015 at 9:00 AM, Michael Paquier
 wrote:
> On Fri, Mar 20, 2015 at 9:48 PM, Bruce Momjian  wrote:
>> On Tue, Oct 28, 2014 at 07:02:41AM +, krystian.bi...@gmail.com wrote:
>>> The following bug has been logged on the website:
>>>
>>> Bug reference:  11805
>>> Logged by:  Krystian Bigaj
>>> Email address:  krystian.bi...@gmail.com
>>> PostgreSQL version: 9.3.5
>>> Operating system:   Windows 7 Pro x64
>>> Description:
>>>
>>> pg_ctl on Windows during service start/shutdown should notify service
>>> manager about it's status by increment dwCheckPoint and call to
>>> SetServiceStatus/pgwin32_SetServiceStatus.
>>>
>>> However during shutdown there is a missing call to SetServiceStatus.
>>> See src\bin\pg_ctl\pg_ctl.c:
>>
>> [ thread moved to hackers ]
>>
>> Can a Windows person look into this issue?
>>
>>
>> http://www.postgresql.org/message-id/20141028070241.2593.58...@wrigleys.postgresql.org
>>
>> The thread includes a patch.  I need a second person to verify its
>> validity.  Thanks.
>
> FWIW, it looks sane to me to do so, ServiceMain declaration is in
> charge to start the service, and to wait for the postmaster to stop,
> and indeed process may increment dwcheckpoint in -w mode, and it
> expects for process to wait for 12 times but this promise is broken.
> The extra calls to SetServiceStatus are also welcome to let the SCM
> know the current status in more details.

So, what's next here?

-- 
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] configure can't detect proper pthread flags

[Robert Haas]

On Wed, Apr 8, 2015 at 2:31 AM, Max Filippov  wrote:
> On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov  wrote:
>> On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov  wrote:
>>> Ok, one more attempt: maybe instead of checking that stderr is empty
>>> we could check that stderr has changed in the presence of the option
>>> that we test?
>>
>> The patch:
>> http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvb...@gmail.com
>
> Ping?
> Are there any issues with that approach and the patch?

I think the thing to do is add your patch here so that it doesn't get
forgotten about:

https://commitfest.postgresql.org/action/commitfest_view/open

-- 
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] Use outerPlanState() consistently in executor code

[Robert Haas]

On Wed, Apr 15, 2015 at 3:38 PM, Qingqing Zhou
 wrote:
> In executor context, outerPlanState(node) is the same as
> node->ss.ps.lefttree. We follow this in most places except a few. This
> patch clean up the outliers and might save us a few instructions by
> removing indirection.
>
> Most of changes are trivial. Except I take out an outerPlan nullable
> check in grouping iterator - as a it surely has a left child.

I don't see any particular reason not to do this.

The patch is weird, though.  If I open it with "less", I get binary
garbage.  My Mac's TextEdit app opens it OK though.

-- 
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] Loss of some parts of the function definition

[Pavel Stehule]

Hi

2015-04-30 13:44 GMT+02:00 Sergey Grinko :

> Hi,
>
> Dear developers, I have a request to you.
>
> Now create a script in the application of its function parameters and
> return values can be declared using %TYPE.
> However, when you save the script is stored inside the server only what is
> considered his body. Thus, we obtain:
> 1) loss of the custom formatting.
> 2) loss of communication parameters and return types with these types of
> fields to create the function.
> 3) multidimensional arrays are transformed into one-dimensional: [][] -> []
> 4) loss of data accuracy: numeric(n,m) -> numeric
>
> Please - how to save and restore the entire text of the definition to
> CREATE END; unchanged.
>

I am afraid, it is not possible

Postgres doesn't distinguish between multidimensional and one dimensional
arrays - multidimensional is just syntax suger, same is function arguments
- Postgres doesn't store precision for parameters. type%TYPE is translated
to target type outside plpgsql function. These informations are not saved,
so you cannot to take it from PostgreSQL

Regards

Pavel Stehule






>
>
> --
> Yours faithfully, Sergey Grinko
> Email: sergey.gri...@gmail.com
>

Re: [HACKERS] BRIN range operator class

[Robert Haas]

On Mon, Apr 6, 2015 at 5:17 PM, Alvaro Herrera  wrote:
> Thanks for the updated patch; I will at it as soon as time allows.  (Not
> really all that soon, regrettably.)
>
> Judging from a quick look, I think patches 1 and 5 can be committed
> quickly; they imply no changes to other parts of BRIN.  (Not sure why 1
> and 5 are separate.  Any reason for this?)  Also patch 2.
>
> Patch 4 looks like a simple bugfix (or maybe a generalization) of BRIN
> framework code; should also be committable right away.  Needs a closer
> look of course.

Is this still pending?

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


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

[HACKERS] feature freeze and beta schedule

[Peter Eisentraut]

The schedule

calls for beta in June.  In light of that, the core team has agreed to
call for

feature freeze on May 15

That means that all patches that add or change features should be
committed by then.

If you have spare cycles, there are a number of relevant patches still
open in the commit fest.


-- 
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] json_populate_record issue - TupleDesc reference leak

[Pavel Stehule]

Still issue is not fixed still

create type pt as (a int, b int);
postgres=# select json_populate_record('(10,20)'::pt, '{}');
WARNING:  TupleDesc reference leak: TupleDesc 0x7f413ca325b0 (16560,-1)
still referenced


2015-04-30 14:32 GMT+02:00 Bruce Momjian :

> On Thu, Feb 26, 2015 at 05:31:44PM -0500, Tom Lane wrote:
> > Andrew Dunstan  writes:
> > > This doesn't look quite right. Shouldn't we unconditionally release the
> > > Tupledesc before the returns at lines 2118 and 2127, just as we do at
> > > the bottom of the function at line 2285?
> >
> > I think Pavel's patch is probably OK as-is, because the tupdesc returned
> > by get_call_result_type isn't reference-counted; but I agree the code
> > would look cleaner your way.  If the main exit isn't bothering to
> > distinguish this then the early exits should not either.
> >
> > What I'm wondering about, though, is this bit at line 2125:
> >
> >   /* same logic as for json */
> >   if (!have_record_arg && rec)
> >   PG_RETURN_POINTER(rec);
> >
> > If that's supposed to be the same logic as in the other path, then how
> > is it that !have_record_arg has anything to do with whether the JSON
> > object is empty?  Either the code is broken, or the comment is.
>
> Where are we on this?
>
> --
>   Bruce Momjian  http://momjian.us
>   EnterpriseDB http://enterprisedb.com
>
>   + Everyone has their own god. +
>

Re: [HACKERS] Reducing tuple overhead

[Robert Haas]

On Thu, Apr 30, 2015 at 8:05 AM, Simon Riggs  wrote:
> A much better idea is to work out how to avoid index bloat at cause. If we
> are running an UPDATE and we cannot get a cleanup lock, we give up and do a
> non-HOT update, causing the index to bloat. It seems better to wait for a
> short period to see if we can get the cleanup lock. The short period is
> currently 0, so lets start there and vary the duration of wait upwards
> proportionally as the index gets more bloated.

What I'd be worried about there is that it would be very hard to tune
the wait time, and that the operating system scheduling granularity
(10ms?) would be way too long.

But I'm in vigorous agreement with you on one point: the solution to
index bloat (and probably heap bloat, too) is not to clean it up
faster but to create less of it in the first place.  Making more
updates HOT is one way to do that.

-- 
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] json_populate_record issue - TupleDesc reference leak

[Bruce Momjian]

On Thu, Feb 26, 2015 at 05:31:44PM -0500, Tom Lane wrote:
> Andrew Dunstan  writes:
> > This doesn't look quite right. Shouldn't we unconditionally release the 
> > Tupledesc before the returns at lines 2118 and 2127, just as we do at 
> > the bottom of the function at line 2285?
> 
> I think Pavel's patch is probably OK as-is, because the tupdesc returned
> by get_call_result_type isn't reference-counted; but I agree the code
> would look cleaner your way.  If the main exit isn't bothering to
> distinguish this then the early exits should not either.
> 
> What I'm wondering about, though, is this bit at line 2125:
> 
>   /* same logic as for json */
>   if (!have_record_arg && rec)
>   PG_RETURN_POINTER(rec);
> 
> If that's supposed to be the same logic as in the other path, then how
> is it that !have_record_arg has anything to do with whether the JSON
> object is empty?  Either the code is broken, or the comment is.

Where are we on this?

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

  + Everyone has their own god. +


-- 
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] Reducing tuple overhead

[Simon Riggs]

On 25 April 2015 at 01:12, Amit Kapila  wrote:

> On Sat, Apr 25, 2015 at 1:58 AM, Jim Nasby 
> wrote:
> >
> > On 4/23/15 10:40 PM, Amit Kapila wrote:
> >>
> >> I agree with you and what I think one of the major reasons of bloat is
> that
> >> Index segment doesn't have visibility information due to which clearing
> of
> >> Index needs to be tied along with heap.  Now if we can move transaction
> >> information at page level, then we can even think of having it in Index
> >> segment as well and then Index can delete/prune it's tuples on it's own
> >> which can reduce the bloat in index significantly and there is a benefit
> >> to Vacuum as well.
> >
> >
> > I don't see how putting visibility at the page level helps indexes at
> all. We could already put XMIN in indexes if we wanted, but it won't help,
> because...
> >
>
> We can do that by putting transaction info at tuple level in index as
> well but that will be huge increase in size of index unless we devise
> a way to have variable index tuple header rather than a fixed.
>
> >> Now this has some downsides as well like Delete
> >> needs to traverse Index segment as well to Delete mark the tuples, but
> >> I think the upsides of reducing bloat can certainly outweigh the
> downsides.
> >
> >
> > ... which isn't possible. You can not go from a heap tuple to an index
> tuple.
>
> We will have the access to index value during delete, so why do you
> think that we need linkage between heap and index tuple to perform
> Delete operation?  I think we need to think more to design Delete
> .. by CTID, but that should be doable.
>

I see some assumptions here that need to be challenged.

We can keep xmin and/or xmax on index entries. The above discussion assumes
that the information needs to be updated synchronously. We already store
visibility information on index entries using the lazily updated killtuple
mechanism, so I don't see much problem in setting the xmin in a similar
lazy manner. That way when we use the index if xmax is set we use it, if it
is not we check the heap. (And then you get to freeze indexes as well ;-( )
Anyway, I have no objection to making index AM pass visibility information
to indexes that wish to know the information, as long as it is provided
lazily.

The second assumption is that if we had visibility information in the index
that it would make a difference to bloat. Since as I mention, we already do
have visibility information, I don't see that adding xmax or xmin would
make any difference at all to bloat. So -1 to adding it **for that reason**.


A much better idea is to work out how to avoid index bloat at cause. If we
are running an UPDATE and we cannot get a cleanup lock, we give up and do a
non-HOT update, causing the index to bloat. It seems better to wait for a
short period to see if we can get the cleanup lock. The short period is
currently 0, so lets start there and vary the duration of wait upwards
proportionally as the index gets more bloated.

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

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

Re: [HACKERS] Disabling trust/ident authentication configure option

[Robert Haas]

On Thu, Apr 16, 2015 at 9:55 AM, Bernd Helmle  wrote:
> PostgreSQL is deployed as part of a larger technical solution (e.g. a
> Telecommunication system) and a field engineer has to install/upgrade this
> solution. The engineer is a specialist in the Telco domain and has only
> little knowledge of databases and especially PostgreSQL setup.
>
> We now want to provide these kinds of users with pre-hardened packages that
> make it very hard to accidentally expose their database. For this purpose
> the patch allows to optionally disable the "trust" and "ident"
> authentication methods. Especially the "trust" mechanism is very critical
> as it might actually provide useful functionality for our user. Think of an
> engineer who has to do a night shift upgrade with a customer breathing down
> his neck to get the system back online. Circumventing all these
> authentication configuration issues by just enabling "trust" is very easy
> and looks well supported and documented.

But... the user could use password authentication with the password
set to "x" and that would be insecure, too, yet not prevented by any
of this.  I think it's pretty hard to prevent someone who has
filesystem-level access to the database server from configuring it
insecurely.

Of course, it's fine for people to make changes like this in their own
copies of PostgreSQL, but I'm not in favor of incorporating those
changes into core.  I don't think there's enough general utility to
this to justify that, and more to the point, I think different people
will want different things.  We haven't, for example, ever had a
request for this specific thing before.

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


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

[HACKERS] Loss of some parts of the function definition

[Sergey Grinko]

Hi,

Dear developers, I have a request to you.

Now create a script in the application of its function parameters and
return values can be declared using %TYPE.
However, when you save the script is stored inside the server only what is
considered his body. Thus, we obtain:
1) loss of the custom formatting.
2) loss of communication parameters and return types with these types of
fields to create the function.
3) multidimensional arrays are transformed into one-dimensional: [][] -> []
4) loss of data accuracy: numeric(n,m) -> numeric

Please - how to save and restore the entire text of the definition to
CREATE END; unchanged.


-- 
Yours faithfully, Sergey Grinko
Email: sergey.gri...@gmail.com

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Robert Haas]

On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko  wrote:
> Attached v10 patch is latest version patch.
> The syntax is,
> REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
>
> That is, WITH clause is optional.

I thought we agreed on moving this earlier in the command:

http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us

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