Re: [HACKERS] Logical decoding (contrib/test_decoding) walsender broken in 9.5 master?
On Thu, Apr 2, 2015 at 5:35 PM, Craig Ringer cr...@2ndquadrant.com wrote: I started bisecting from e6df2e1 (stamp 9.4beta1, good) to 095d401 (bad). The problem revision appears to be 9402869: commit 9402869 Author: Heikki Linnakangas heikki.linnakan...@iki.fi Date: Sat Jan 17 01:14:32 2015 +0200 Advance backend's advertised xmin more aggressively. This rings a bell, I reported a similar issue some weeks back: http://www.postgresql.org/message-id/CAB7nPqQSdx7coHk0D6G=mkjntgyjxpdw+pwiskkssaezfts...@mail.gmail.com And there is a patch. -- 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] PL/pgSQL, RAISE and error context
2015-01-26 16:46 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com: 2015-01-26 16:14 GMT+01:00 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja ma...@joh.to: I am thinking, so solution /* if we are doing RAISE, don't report its location */ if (estate-err_text == raise_skip_msg) return; is too simple, and this part should be fixed. This change can be done by on plpgsql or libpq side. This is bug, and it should be fixed. Doing this in libpq is utterly insane. It has not got sufficient context to do anything intelligent. The fact that it's not intelligent is exposed by the regression test changes that the proposed patch causes, most of which do not look like improvements. I don't understand. There can be a overhead due useless transformation some data to client side. But all what it need - errcontext and errlevel is possible. Another problem is that past requests to change this behavior have generally been to the effect that people wanted *more* context suppressed not less, ie they didn't want any CONTEXT lines at all on certain messages. So the proposed patch seems to me to be going in exactly the wrong direction. The design I thought had been agreed on was to add some new option to plpgsql's RAISE command which would cause suppression of all CONTEXT lines not just the most closely nested one. You could argue about whether the behavior needs to be 100% backwards compatible or not --- if so, perhaps it could be a three-way option all, none, or one line, defaulting to the last for backwards compatibility. I see a problem what should be default behave. When I raise NOTICE, then I don't need (don't would) to see CONTEXT lines, When I raise EXCEPTION, then I usually would to see CONTEXT lines. Cannot be solution? I would to wakeup this thread. estate-err_text = stmt-elog_level == ERROR ? estate-err_text : raise_skip_msg ; Can we do this simple change? It will produce a stackinfo for exceptions and it will not to make mad developers by lot of useless content. Regards Pavel Regards Pavel regards, tom lane
Re: [HACKERS] Logical decoding (contrib/test_decoding) walsender broken in 9.5 master?
I started bisecting from e6df2e1 (stamp 9.4beta1, good) to 095d401 (bad).
The problem revision appears to be 9402869:
commit 9402869
Author: Heikki Linnakangas heikki.linnakan...@iki.fi
Date: Sat Jan 17 01:14:32 2015 +0200
Advance backend's advertised xmin more aggressively.
which is somewhat logical given the crash location and symptoms.
Bisection procedure used:
For each step:
./configure --enable-debug --enable-cassert --prefix=/home/craig/pg/95
CFLAGS=-Og -ggdb -fno-omit-frame-pointer make clean install make -C
contrib/test_decoding/ clean install rm -rf ~/tmp/slottest95
PATH=$HOME/pg/95/bin:$PATH initdb -D ~/tmp/slottest95 cp
~/tmp/{postgresql.conf,pg_hba.conf} ~/tmp/slottest95
PATH=$HOME/pg/95/bin:$PATH PGPORT=5123 postgres -D ~/tmp/slottest95/
then manually:
psql -p 5123 -c 'SELECT pg_create_logical_replication_slot('test',
'test_decoding');'
PGPORT=5123 PATH=$HOME/pg/95/bin:$PATH pg_recvlogical -d postgres -S test
--start -f -
psql -p 5123 -c 'CREATE TABLE x AS SELECT xx FROM generate_series(1,1)
xx;'
I have to go out now; I'll follow up further but wanted to update promptly.
Re: [HACKERS] Parallel Seq Scan
On Wed, Apr 1, 2015 at 8:18 PM, Robert Haas robertmh...@gmail.com wrote:
I don't think you need to do anything that complicated. I'm not
proposing to *run* the initPlan in the workers, just to pass the
parameter values down.
Sorry, but I am not able to understand how it will help if just
parameters
(If I understand correctly you want to say about Bitmapset *extParam;
Bitmapset *allParam; in Plan structure) are passed to workers and I
think they are already getting passed only initPlan and related Subplan
in planned statement is not passed and the reason is that
ss_finalize_plan()
attaches initPlan to top node (which in this case is Funnel node and not
PartialSeqScan)
By any chance, do you mean that we run the part of the statement in
workers and then run initPlan in master backend?
If I'm not confused, it would be the other way around. We would run
the initPlan in the master backend *first* and then the rest in the
workers.
Either one of us is confused, let me try to describe my understanding in
somewhat more detail. Let me try to explain w.r.t the tab completion
query [1]. In this, the initPlan is generated for Qualification expression
[2], so it will be executed during qualification and the callstack will
look like:
postgres.exe!ExecSeqScan(ScanState * node=0x0c33bce8) Line 113 C
postgres.exe!ExecProcNode(PlanState * node=0x0c33bce8) Line 418
+ 0xa bytes C
postgres.exe!ExecSetParamPlan(SubPlanState * node=0x0c343930,
ExprContext * econtext=0x0c33de50) Line 1001 + 0xa bytes C
postgres.exe!ExecEvalParamExec(ExprState * exprstate=0x0c33f980,
ExprContext * econtext=0x0c33de50, char *
isNull=0x0c33f481, ExprDoneCond * isDone=0x) Line
C
postgres.exe!ExecMakeFunctionResultNoSets(FuncExprState *
fcache=0x0c33f0d0, ExprContext * econtext=0x0c33de50, char
* isNull=0x0042f1c8, ExprDoneCond * isDone=0x)
Line 1992 + 0x2d bytes C
postgres.exe!ExecEvalOper(FuncExprState * fcache=0x0c33f0d0,
ExprContext * econtext=0x0c33de50, char *
isNull=0x0042f1c8, ExprDoneCond * isDone=0x) Line
2443 C
postgres.exe!ExecQual(List * qual=0x0c33fa08, ExprContext *
econtext=0x0c33de50, char resultForNull=0) Line 5206 + 0x1a bytes C
postgres.exe!ExecScan(ScanState * node=0x0c33dd38, TupleTableSlot
* (ScanState *)* accessMtd=0x000140232940, char (ScanState *,
TupleTableSlot *)* recheckMtd=0x0001402329e0) Line 195 + 0x1a bytes C
postgres.exe!ExecSeqScan(ScanState * node=0x0c33dd38) Line 114 C
Basically here initPlan is getting executed during Qualification.
So now the point I am not able to understand from your explanation
is that how the worker will perform qualification without the knowledge
of initPlan?
[1]
SELECT pg_catalog.quote_ident(c.relname) FROM pg_catalog.pg_class c WHERE
c.relkind IN ('r', 'S', 'v', 'm',
'f') AND substring(pg_catalog.quote_ident(c.relname),1,3)='pgb' AND
pg_catalog.pg_table_is_visible(c.oid) AND
c.relnamespace (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname =
'pg_catalog') UNION SELECT
pg_catalog.quote_ident(n.nspname) || '.' FROM pg_catalog.pg_namespace n
WHERE substring
(pg_catalog.quote_ident(n.nspname) || '.',1,3)='pgb' AND (SELECT
pg_catalog.count(*) FROM
pg_catalog.pg_namespace WHERE substring(pg_catalog.quote_ident(nspname) ||
'.',1,3) = substring
('pgb',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) 1 UNION
SELECT pg_catalog.quote_ident
(n.nspname) || '.' || pg_catalog.quote_ident(c.relname) FROM
pg_catalog.pg_class c, pg_catalog.pg_namespace n
WHERE c.relnamespace = n.oid AND c.relkind IN ('r', 'S', 'v', 'm', 'f') AND
substring(pg_catalog.quote_ident
(n.nspname) || '.' || pg_catalog.quote_ident(c.relname),1,3)='pgb' AND
substring(pg_catalog.quote_ident
(n.nspname) || '.',1,3) =
substring('pgb',1,pg_catalog.length(pg_catalog.quote_ident(n.nspname))+1)
AND
(SELECT pg_catalog.count(*) FROM pg_catalog.pg_namespace WHERE
substring(pg_catalog.quote_ident(nspname) ||
'.',1,3) =
substring('pgb',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) =
1 LIMIT 1000;
[2]
(SELECT pg_catalog.count(*) FROM pg_catalog.pg_namespace WHERE
substring(pg_catalog.quote_ident(nspname) ||
'.',1,3) =
substring('pgb',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] Parallel Seq Scan
On Wed, Apr 1, 2015 at 6:11 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Apr 1, 2015 at 7:30 AM, Amit Kapila amit.kapil...@gmail.com wrote: Also, the new code to propagate XactLastRecEnd won't work right, either. As we are generating FATAL error on termination of worker (bgworker_die()), so won't it be handled in AbortTransaction path by below code in parallel-mode patch? That will asynchronously flush the WAL, but if the master goes on to commit, we've wait synchronously for WAL flush, and possibly sync rep. Okay, so you mean if master backend later commits, then there is a chance of loss of WAL data written by worker. Can't we report the location to master as the patch does in case of Commit in worker? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] libpq's multi-threaded SSL callback handling is busted
Peter Eisentraut writes: On 2/12/15 7:28 AM, Jan Urbański wrote: For the sake of discussion, here's a patch to prevent stomping on previously-set callbacks, racy as it looks. FWIW, it does fix the Python deadlock and doesn't cause the PHP segfault... I don't think this patch would actually fix the problem that was described after the original bug report (http://www.postgresql.org/message-id/5436991b.5020...@vmware.com), namely that another thread acquires a lock while the libpq callbacks are set and then cannot release the lock if libpq has been shut down in the meantime. I did test both the Python and the PHP repro scripts and the patch fixed both the deadlock and the segfault. What happens is that Python (for instance) stops over the callback unconditionally. So when libpq gets unloaded, it sees that the currently set callback is no the one it originally set and refrains from NULLing it. There's a small race window there, to be sure, but it's a lot better than what we have now. Cheers, Jan -- 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] What exactly is our CRC algorithm?
At 2015-03-25 19:18:51 +0200, hlinn...@iki.fi wrote:
I think we'll need a version check there. […]
You want to write that or should I?
I'm not familiar with MSVC at all, so it would be nice if you did it.
How do you like this latest version of the patch otherwise?
I'm sorry, but I'm still not especially fond of it.
Apart from removing the startup check so that client programs can also
use the best available CRC32C implementation without jumping through
hoops, I don't feel that the other changes buy us very much.
Also, assuming that the point is that people who don't care about CRCs
deeply should nevertheless be able to produce special-purpose binaries
with only the optimal implementation included, we should probably have
some instructions about how to do that.
Thinking about what you were trying to do, I would find an arrangement
roughly like the following to be clearer to follow in terms of adding
new implementations and so on:
#if defined(USE_CRC_SSE42) || …can build SSE4.2 CRC code…
#define HAVE_CRC_SSE42 1
pg_crc32c_sse42() { … }
bool sse42_crc32c_available() { … }
pg_comp_crc32c = pg_crc32c_sse42;
#elif defined(USE_CRC_ARM) || …can build ARM CRC code…
#define HAVE_CRC_ARM 1
pg_crc32c_arm() { … }
bool arm_crc32c_available() { … }
pg_comp_crc32c = pg_crc32c_arm;
#endif
#define CRC_SELECTION 1
#if defined(USE_CRC_SSE42) || defined(USE_CRC_ARM) || …
#undef CRC_SELECTION
#endif
#ifdef CRC_SELECTION
pg_crc32c_sb8() { … }
pg_comp_crc32c_choose(…)
{
pg_comp_crc32c = pg_crc32c_sb8;
#ifdef HAVE_CRC_SSE42
if (sse42_crc32c_available())
pg_comp_crc32c = pg_crc32c_sse42;
#elif …
…
#endif
return pg_comp_crc32c(…);
}
pg_comp_crc32c = pg_crc32c_choose;
#endif
Then someone who wants to force the building of (only) the SSE4.2
implementation can build with -DUSE_CRC_SSE42. And if you turn on
USE_CRC_ARM when you can't build ARM code, it won't build. (The
HAVE_CRC_xxx #defines could also move to configure tests.)
If you don't specify any USE_CRC_xxx explicitly, then it'll build
whichever (probably) one it can, and select it at runtime if it's
available.
All that said, I do recognise that there are all relatively cosmetic
concerns, and I don't object seriously to any of it. On the contrary,
thanks for taking the time to review and work on the patch. Nobody
else has expressed an opinion, so I'll leave it to you to decide whether
to commit as-is, or if you want me to pursue the above approach instead.
In the realm of very minor nitpicking, here are a couple of points I
noticed in crc_instructions.h:
1. I think «if (pg_crc32_instructions_runtime_check())» would read
better if the function were named crc32c_instructions_available or
pg_crc32c_is_hw_accelerated, or something like that.
2. It documents pg_accumulate_crc32c_byte and
pg_accumulate_crc32c_uint64, but actually defines pg_asm_crc32b and
pg_asm_crc32q. If you update the code rather than the documentation,
_update_ may be slightly preferable to _accumulate_, and maybe the
suffixes should be _uint8 and _uint64.
3. The descriptions (e.g. Add one byte to the current crc value.)
should also probably read Update the current crc value for the given
byte/eight bytes of data.
Thanks.
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Logical decoding (contrib/test_decoding) walsender broken in 9.5 master?
Hi all
It appears that logical decoding may be broken in 9.5 at the moment.
With HEAD at f6caf5a:
./configure --enable-debug --enable-cassert --prefix=/home/craig/pg/95
CFLAGS=-Og -ggdb -fno-omit-frame-pointer
make clean install
make -C contrib/test_decoding clean install
PGPORT=5142 PATH=/home/craig/pg/95/bin:$PATH initdb -D ~/tmp/slottest95
PGPORT=5142 PATH=/home/craig/pg/95/bin:$PATH postgres -D ~/tmp/slottest95
and in another session:
psql -p 5142 -c 'SELECT pg_create_logical_replication_slot('test',
'test_decoding');'
in yet another:
PGPORT=5142 PATH=$HOME/pg/95/bin:$PATH pg_recvlogical -d postgres -S test
--start -f -
and back in the psql session do some work:
psql -p 5142 -c 'CREATE TABLE x AS SELECT xx FROM generate_series(1,1)
xx;'
This works fine in REL9_4_STABLE at a44e54c.
Decoding over the SQL protocol works fine, and make check in
contrib/test_decoding passes without errors. This issue only arises in
decoding in a walsender.
I haven't bisected it back to a specific change yet, I just wanted to give
early heads-up. Also, our testing clearly needs to cover logical decoding
over walsenders.
See attachment for the bt.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
Core was generated by `postgres: wal sender pr'.
Program terminated with signal SIGABRT, Aborted.
#0 0x7f4005aa28d7 in __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:55
55return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
(gdb) bt
#0 0x7f4005aa28d7 in __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:55
#1 0x7f4005aa453a in __GI_abort () at abort.c:89
#2 0x008e3a00 in ExceptionalCondition (conditionName=0xae3528
!(!((RegisteredSnapshots)-ph_root == ((void *)0))), errorType=0xae323a
FailedAssertion, fileName=0xae3230 snapmgr.c, lineNumber=677) at assert.c:54
#3 0x00929082 in UnregisterSnapshotFromOwner (snapshot=0x7f3ff5232f58,
owner=0x1cd4698) at snapmgr.c:677
#4 0x0092901b in UnregisterSnapshot (snapshot=0x7f3ff5232f58) at
snapmgr.c:663
#5 0x004c4b76 in systable_endscan (sysscan=0x1d1b210) at genam.c:504
#6 0x008cf3d7 in RelationBuildTupleDesc (relation=0x7f40066ffe78) at
relcache.c:569
#7 0x008d055c in RelationBuildDesc (targetRelId=3455, insertIt=1
'\001') at relcache.c:1036
#8 0x008d221c in RelationIdGetRelation (relationId=3455) at
relcache.c:1778
#9 0x004aad94 in relation_open (relationId=3455, lockmode=1) at
heapam.c:1047
#10 0x004c4efb in index_open (relationId=3455, lockmode=1) at
indexam.c:167
#11 0x004c45ff in systable_beginscan (heapRelation=0x7f40067584e0,
indexId=3455, indexOK=1 '\001', snapshot=0x0, nkeys=2, key=0x7ffcde1e7900) at
genam.c:334
#12 0x008da269 in RelidByRelfilenode (reltablespace=0,
relfilenode=1247) at relfilenodemap.c:204
#13 0x00750dd7 in ReorderBufferCommit (rb=0x1c62788, xid=1865,
commit_lsn=25360464, end_lsn=25360872, commit_time=481271227439738) at
reorderbuffer.c:1338
#14 0x0074bac7 in DecodeCommit (ctx=0x1c60778, buf=0x7ffcde1e7c80,
parsed=0x7ffcde1e7bc0, xid=1865) at decode.c:494
#15 0x0074b43a in DecodeXactOp (ctx=0x1c60778, buf=0x7ffcde1e7c80) at
decode.c:211
#16 0x0074b1a7 in LogicalDecodingProcessRecord (ctx=0x1c60778,
record=0x1c60a38) at decode.c:100
#17 0x0075ad78 in XLogSendLogical () at walsender.c:2425
#18 0x0075a078 in WalSndLoop (send_data=0x75acd7 XLogSendLogical) at
walsender.c:1834
#19 0x007590f0 in StartLogicalReplication (cmd=0x1c59eb8) at
walsender.c:997
#20 0x00759713 in exec_replication_command (cmd_string=0x1cb3108
START_REPLICATION SLOT \test\ LOGICAL 0/0) at walsender.c:1326
#21 0x007aea57 in PostgresMain (argc=1, argv=0x1c40350,
dbname=0x1c40240 postgres, username=0x1c40220 craig) at postgres.c:4021
#22 0x007354e3 in BackendRun (port=0x1c64900) at postmaster.c:4141
#23 0x00734c4b in BackendStartup (port=0x1c64900) at postmaster.c:3826
#24 0x0073150a in ServerLoop () at postmaster.c:1594
#25 0x00730b95 in PostmasterMain (argc=3, argv=0x1c3f410) at
postmaster.c:1241
#26 0x00690e64 in main (argc=3, argv=0x1c3f410) at main.c:221
(gdb) bt full
#0 0x7f4005aa28d7 in __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:55
resultvar = 0
pid = 22806
selftid = 22806
#1 0x7f4005aa453a in __GI_abort () at abort.c:89
save_stage = 2
act = {__sigaction_handler = {sa_handler = 0x0, sa_sigaction = 0x0},
sa_mask = {__val = {140724035024416, 4590256, 139912960889648, 11420318053453,
72057594037927936, 1, 30519912, 140724035024352, 9497950, 72057594068447848,
2520, 0, 139912950442304, 0, 139912953362992, 139912953358080}}, sa_flags =
98728496, sa_restorer = 0x7f4006734700}
sigs = {__val = {32, 0 repeats 15 times}}
#2
[HACKERS] Supporting TAP tests with MSVC and Windows
Hi all,
(Adding Peter and Heikki in CC for awareness)
Please find attached a WIP patch to support TAP tests with MSVC. The
tests can be kicked with this command:
vcregress tapcheck
There are a couple of things to note with this patch, and I would like
to have some input for some of those things:
1) I have tweaked some code paths to configure a node correctly, with
for example listen_addresses = 'localhost', or disabling local
settings in pg_hba.conf. Does that sound fine?
2) tapcheck does not check if IPC::Run is installed or not. Should we
add a check in the code for that? If yes, should we bypass the test or
fail?
3) Temporary installation needs to be done in the top directory,
actually out of src/, because Install.pm scans the contents of src/ to
fetch for example the .samples files and this leads to bad
interactions. Using this location has as well the advantage to install
the binaries for all the tests only once.
4) pg_rewind tests are disabled for now, I am waiting for the outcome
of 5519f169.8030...@gmx.net
5) ssl tests are disabled for now (haven't looked at that yet). They
should be tested if possible. Still need some investigation.
6) the command to access pg_regress is passed using an environment
variable TESTREGRESS.
7) TAP tests use sometimes top_builddir directly. I think that's ugly,
and if there are no objections I think that we should change it to use
a dedicated environment variable like TESTTOP or similar.
8) Some commands have needed some tweaks because option parsing on
Windows is... Well... sometimes broken. For example those things do
not work on Windows:
createdb foobar3 -E win1252
initdb PGDATA -X XLOGDIR
Note that modifying those commands does not change the test coverage.
9) @Peter: IPC::Run is not reliable when using h-results, by
switching to (h-full_results)[0] I was able to get results reliably
from a child process.
10) This patch needs to have IPC::Run installed. You need to install
it from CPAN, that's the same deal as for OSX.
11) Windows does not support symlink, so some tests of pg_basebackup
cannot be executed. I can live with this restriction.
I think that's all for now. I'll add this patch to the 2015-06 commit fest.
Regards,
--
Michael
diff --git a/.gitignore b/.gitignore
index 8d3af50..3cd37fe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,3 +36,6 @@ lib*.pc
/pgsql.sln.cache
/Debug/
/Release/
+
+# Generated by tests
+/tmp_check/
diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 9b77648..d3d8f5f 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -438,6 +438,7 @@ $ENV{CONFIG}=Debug;
userinputvcregress contribcheck/userinput
userinputvcregress ecpgcheck/userinput
userinputvcregress isolationcheck/userinput
+userinputvcregress tapcheck/userinput
userinputvcregress upgradecheck/userinput
/screen
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 149b3d1..091ec0e 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -1,10 +1,26 @@
use strict;
use warnings;
+use Config;
use TestLib;
use Test::More tests = 19;
my $tempdir = TestLib::tempdir;
+sub cleanup_tempdir
+{
+ my $tempdir = shift;
+
+ if ($Config{osname} eq MSWin32)
+ {
+ system_or_bail rd /s /q $tempdir;
+ mkdir $tempdir;
+ }
+ else
+ {
+ system_or_bail rm -rf '$tempdir'/*;
+ }
+}
+
program_help_ok('initdb');
program_version_ok('initdb');
program_options_handling_ok('initdb');
@@ -18,27 +34,26 @@ 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'/*;
-
-command_ok([ 'initdb', $tempdir/data, '-X', $tempdir/pgxlog ],
- 'separate xlog directory');
+cleanup_tempdir $tempdir;
-system_or_bail rm -rf '$tempdir'/*;
+command_ok([ 'initdb', '-D', $tempdir/data, '-X', $tempdir/pgxlog ],
+ 'separate xlog directory');
+cleanup_tempdir $tempdir;
command_fails(
[ 'initdb', $tempdir/data, '-X', 'pgxlog' ],
'relative xlog directory not allowed');
-system_or_bail rm -rf '$tempdir'/*;
+cleanup_tempdir $tempdir;
mkdir $tempdir/pgxlog;
-command_ok([ 'initdb', $tempdir/data, '-X', $tempdir/pgxlog ],
+command_ok([ 'initdb', '-D', $tempdir/data, '-X', $tempdir/pgxlog ],
'existing empty xlog directory');
-system_or_bail rm -rf '$tempdir'/*;
+cleanup_tempdir $tempdir;
mkdir $tempdir/pgxlog;
mkdir $tempdir/pgxlog/lost+found;
-command_fails([ 'initdb', $tempdir/data, '-X', $tempdir/pgxlog ],
+command_fails([ 'initdb', '-D', $tempdir/data, '-X', $tempdir/pgxlog ],
'existing nonempty xlog directory');
-system_or_bail rm -rf '$tempdir'/*;
-command_ok([ 'initdb', $tempdir/data, '-T', 'german' ],
+cleanup_tempdir $tempdir;
+command_ok([ 'initdb', '-D', $tempdir/data, '-T', 'german' ],
'select default dictionary');
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
Re: [HACKERS] Logical decoding (contrib/test_decoding) walsender broken in 9.5 master?
On Thu, Apr 2, 2015 at 3:48 PM, Craig Ringer cr...@2ndquadrant.com wrote: Also, our testing clearly needs to cover logical decoding over walsenders. Noted. -- 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] Tables cannot have INSTEAD OF triggers
auto-updatable view work just for postgresql-9.3 and above (for other version
you still need to define DELETE/UPDATE trigger).
what i see is we just trying to have a work around either with BEFORE/AFTER
trigger or with auto-updatable view
in stright forwad/normale way is just to define INSTEAD OF trigger on the
master table that return NEW so it doesn't break RETURNING, and the actual
tuples returned by the trigger wouldn't actually be inserted in the master
table. after all, that what INSTEAD OF suppose to do.
tom lane : in partitioned table. normally (always), the data is stored in child
tables (i know this not the case for inheritence) . any data inserted in master
table is just an exception/error/design bug or this is just my case. what i
mean is, if some one define master table as empty table (even without having
INSTEAD OF trigger) is not wart (postgresql need to be more flexible, and let
user define thier database architecture the way they like).
also, it would be nice that the example :
INSERT INTO cities (name, population, altitude, state)
VALUES ('New York', NULL, NULL, 'NY');
in the inheritence doc to work, (if we maked passes syntax error checking and
planning phase) next step is to chose between rule and trigger (we already
have instead of rule. we just need instead of trigger ) maybe this not a user
defined one but implicitly.
-Original Message-
From: Dean Rasheed dean.a.rash...@gmail.com
To: Andres Freund and...@anarazel.de
Cc: Tom Lane t...@sss.pgh.pa.us; Robert Haas robertmh...@gmail.com; Aliouii
Ali aliouii@aol.fr; pgsql-hackers pgsql-hackers@postgresql.org
Sent: Wed, Apr 1, 2015 8:01 pm
Subject: Re: [HACKERS] Tables cannot have INSTEAD OF triggers
On 1 April 2015 at 18:37, Andres Freund and...@anarazel.de wrote:
On
2015-04-01 13:29:33 -0400, Tom Lane wrote:
As for partitioning, you could do
this:
create table parent(...);
create table child(...)
inherits(parent); -- repeat as needed
create view v as select * from
parent;
attach INSTEAD OF triggers to v
Now the application deals
only with v, and thinks that's the real
table.
Sure, but that's just
making things unnecessarily hard. That then
requires also defining
UPDATE/DELETE INSTEAD triggers which otherwise
would just work.
No,
because as defined above the view v would be auto-updatable, so
updates and
deletes on v would just do the matching update/delete
on
parent.
Regards,
Dean
[HACKERS] GSoC 2015. Support for microvacuum for GiST. Feedback for my proposal.
Hi, all! I have sent GSoC proposal (http://postgresql.nabble.com/GSoC-2015-proposal-Support-for-microvacuum-for-GiST-td5843638.html) about week ago and I haven't received any feedback. So I'm a bit confused. Is the idea of proposal not actual for community or maybe too small for GSoC project? I do want participate in GSoC, but without comments I can't understand what's wrong and improve the proposal. Sincerely, Ilya Ivanitskiy.
Re: [HACKERS] improve pgbench syntax error messages
On Sun, Mar 29, 2015 at 2:20 PM, Fabien COELHO coe...@cri.ensmp.fr wrote: Here is a v5. Here is v6, just a rebase. Committed with minor stylistic fixes. -- 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] Re: Abbreviated keys for Datum tuplesort
On Thu, Apr 2, 2015 at 8:17 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Feb 20, 2015 at 3:57 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: I've spent a fair amount of testing this today, and when using the simple percentile_disc example mentioned above, I see this pattern: master patched speedup - generate_series(1,100) 4.2 0.7 6 generate_series(1,200) 9.2 9.8 0.93 generate_series(1,300) 14.5 15.3 0.95 so for a small dataset the speedup is very nice, but for larger sets there's ~5% slowdown. Is this expected? I think it's explained by the pre-check for sorted input making the number of comparisons exactly n -1. As I pointed out to Tomas, if you put a single, solitary unsorted element at the end, the abbreviated version is then 8x faster (maybe that was in relation to a slightly different case, but the pattern is the same). So this case isn't an argument against datum abbreviation, or even abbreviation in general, but rather an argument against using strxfrm() in general (which for example the GCC docs strongly recommend for sorting lists of strings). It's a bad argument, IMV. This sort is already extremely fast. BTW, Corey Huinker (who I believe you also spoke to during pgConf.US) reported that his problematic CREATE INDEX case was 12x faster with 9.5. That used tapesort. That was also perfectly pre-sorted, but since it was using tapesort and abbreviated there was a huge improvement. I had a look at this patch today with a view to committing it, but it seems that nobody's commented on this point, which seems like an important one. Any thoughts? For what it's worth, and without wishing to provoke another flamewar, I am inclined to use Andrew's version of this patch rather than Peter's. I have not undertaken an exhaustive comparison, nor do I intend to. It is the reviewer's responsibility to justify the changes they think the author needs to make, and that wasn't done here. On the points of difference Andrew highlighted, I think his version is fine. The justification is that Andrew's version had arbitrary differences with the existing code, in particular in the datum comparator, which was different to all other cases in the way that Andrew pointed out. Overall, there were only tiny differences between the two versions. I see no reason to not match the existing style. The changes that Andrew took issue with are utterly insignificant. Also, the changes that Andrew didn't mention are clearly appropriate. In particular, the comments on the SortKeys variable being used by every case except the hash case and datum case should still be updated to reflect that that's only true for the hash case now. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Abbreviated keys for Numeric
On 02/04/15 21:21, Robert Haas wrote:
On Thu, Apr 2, 2015 at 2:07 PM, Robert Haas robertmh...@gmail.com wrote:
I think this is really nice work, so I have committed this version. I
made a few fairly minor changes, hopefully without breaking anything
in the process:
- I adjusted things for recent commits around INT{32,63}_{MIN_MAX}.
- I removed (void) ssup; which I do not think is normal PostgreSQL style.
- Removed the #if DEC_DIGITS != 4 check. The comment is great, but I
think we don't need protect against #if 0 code get re-enabled.
- I removed the too-clever (at least IMHO) handing of TRACE_SORT in
favor of just using #ifdef around each occurrence.
- I also declared trace_sort in guc.h, where various other GUCs are
declared, instead of declaring it privately in each file that needed
it.
- Changed some definitions to depend on SIZEOF_DATUM rather than
USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please check it.
- Fixed an OID conflict.
- And of course, bumped catversion.
And that's broken the buildfarm. Argh.
That's because of this:
+#ifdef SIZEOF_DATUM == 8
You need just #if there.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Tables cannot have INSTEAD OF triggers
On Thu, Apr 2, 2015 at 5:02 PM, Andres Freund and...@anarazel.de wrote: I think the upshot is that INSTEAD OF triggers work in a particular way because that's what is needed to support updatable views. If triggers on tables should behave differently, maybe it should be a separate trigger type. Maybe it would be feasible to extend BEFORE triggers to support RETURNING, for example? What in the above prohibits extending the behaviour to tables? I have yet to see what compatibility or similarity problem that'd pose. It seems all mightily handwavy to me. Yeah. It's possible there's a better interface here than INSTEAD OF, and one of the things I didn't like about the OP was that it started by stating the syntax that would be used rather than by describing the problem that needed to be solved. It's generally better to start with the latter, and then work out the syntax from there. But having gotten that gripe out of my system, and especially in view of Dean's comments, it's not very clear to me what's wrong with using INSTEAD OF for this purpose. If you make BEFORE triggers do this via RETURNING, then you might have a trigger that returns multiple rows, which seems like it would introduce a bunch of new complexity for no obvious benefit. -- 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] Re: Abbreviated keys for Datum tuplesort
On Thu, Apr 2, 2015 at 5:34 PM, Peter Geoghegan p...@heroku.com wrote: I think it's explained by the pre-check for sorted input making the number of comparisons exactly n -1. As I pointed out to Tomas, if you put a single, solitary unsorted element at the end, the abbreviated version is then 8x faster (maybe that was in relation to a slightly different case, but the pattern is the same). So this case isn't an argument against datum abbreviation, or even abbreviation in general, but rather an argument against using strxfrm() in general (which for example the GCC docs strongly recommend for sorting lists of strings). It's a bad argument, IMV. This sort is already extremely fast. OK, I see. The changes that Andrew took issue with are utterly insignificant. Great. Then you will be utterly indifferent to which version gets committed. Also, the changes that Andrew didn't mention are clearly appropriate. In particular, the comments on the SortKeys variable being used by every case except the hash case and datum case should still be updated to reflect that that's only true for the hash case now. On that point, I agree. -- 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] Abbreviated keys for Numeric
On Thu, Apr 2, 2015 at 4:24 PM, Petr Jelinek p...@2ndquadrant.com wrote: Thanks. I actually pushed a fix for that about 25 minutes ago; hopefully that is all that is needed. Ok, the git.pg.org was somewhat behind. It did fix it for me when I tested it locally. OK, that's good to know. So far the buildfarm looks happy too, but I'll try to keep an eye on it at least for the next hour or so. -- 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] What exactly is our CRC algorithm?
On Thu, Apr 2, 2015 at 5:33 PM, Heikki Linnakangas hlinn...@iki.fi wrote: It was added in gcc 4.2. That's good enough for me. I think it's fine to have optional optimizations that require gcc = 4.2, as long as older platforms don't break outright. We have a buildfarm animal that still uses gcc 2.95.3, which was released in 2001. I don't have a compiler of that vintage to test with, but I assume an old enough assembler would not know about the crc32q instruction and fail to compile. GCC from 2002 wouldn't support the symbolic operand names in inline assembly. binutils from 2007 (IIRC) wouldn't support the assembler instructions themselves. We could work around the latter by using the appropriate sequence of bytes. We could work around the former by using the old syntax for operands. I'm OK with not supporting the new instructions when building with an old compiler/assembler. But the build shouldn't fail with an old compiler/assembler. Using old syntax or raw bytes just to avoid failing on an ancient compiler seems ugly. I dunno about old syntax, but raw bytes seems like a bad idea, for sure. -- 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] Abbreviated keys for Numeric
On 02/04/15 22:22, Robert Haas wrote:
On Thu, Apr 2, 2015 at 4:21 PM, Petr Jelinek p...@2ndquadrant.com wrote:
On 02/04/15 21:21, Robert Haas wrote:
On Thu, Apr 2, 2015 at 2:07 PM, Robert Haas robertmh...@gmail.com wrote:
I think this is really nice work, so I have committed this version. I
made a few fairly minor changes, hopefully without breaking anything
in the process:
- I adjusted things for recent commits around INT{32,63}_{MIN_MAX}.
- I removed (void) ssup; which I do not think is normal PostgreSQL style.
- Removed the #if DEC_DIGITS != 4 check. The comment is great, but I
think we don't need protect against #if 0 code get re-enabled.
- I removed the too-clever (at least IMHO) handing of TRACE_SORT in
favor of just using #ifdef around each occurrence.
- I also declared trace_sort in guc.h, where various other GUCs are
declared, instead of declaring it privately in each file that needed
it.
- Changed some definitions to depend on SIZEOF_DATUM rather than
USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please check it.
- Fixed an OID conflict.
- And of course, bumped catversion.
And that's broken the buildfarm. Argh.
That's because of this:
+#ifdef SIZEOF_DATUM == 8
You need just #if there.
Thanks. I actually pushed a fix for that about 25 minutes ago;
hopefully that is all that is needed.
Ok, the git.pg.org was somewhat behind. It did fix it for me when I
tested it locally.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Table-level log_autovacuum_min_duration
Michael Paquier wrote:
So, attached are two patches:
- 0001 enables SIGHUP tracking in do_autovacuum(), which is checked
before processing one table. I reused avl_sighup_handler for the
worker, renaming it av_sighup_handler..
- 0002 is the patch to add log_autovacuum_min_duration as a reloption.
There is nothing really changed, the value of
log_autovacuum_min_duration being picked up at startup with
table_recheck_autovac() is used until the end for one table.
Thanks.
You added this in the worker loop processing each table:
/*
* Check for config changes before processing each collected
table.
*/
if (got_SIGHUP)
{
got_SIGHUP = false;
ProcessConfigFile(PGC_SIGHUP);
/* shutdown requested in config file? */
if (!AutoVacuumingActive())
break;
}
I think bailing out if autovac is disabled is a good idea; for instance,
if this is a for-wraparound vacuum, we should keep running. My first
reaction is that this part of the test ought to be moved down a
screenful or so, until we've ran the recheck step and we have the
for-wraparound flag in hand; only bail out if that one is not set. But
on the other hand, maybe the worker will process some tables that are
not for-wraparound in between some other tables that are, so that might
turn out to be a bad idea as well. (Though I'm not 100% that it works
that way; consider commit f51ead09df for instance.) Maybe the test to
use for this is something along the lines of if autovacuum was enabled
before and is no longer enabled, bail out, otherwise keep running.
This implies that we need to keep track of whether autovac was enabled
at the start of the worker run.
Another thing, but not directly related to this patch: while looking at
the doc changes, I noticed that we don't have index entries for the
reloptions we have; for instance, the word fillfactor doesn't appear
in the bookindex.html page at all. Having these things all crammed in
the CREATE TABLE page seems somewhat poor. I think we could improve on
this by having a listing of settable parameters in a separate section,
and have CREATE TABLE, ALTER TABLE, and the Server Configuration chapter
link to that; we could also have index entries for these items.
Of course, the simpler fix is to just add a few indexterm tags.
As a note, there is no point to Assert(foo != NULL) tests when foo is
later dereferenced in the same routine: the crash is going to happen
anyway at the dereference, so it's just a LOC uselessly wasted.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Abbreviated keys for Numeric
On Thu, Apr 2, 2015 at 4:21 PM, Petr Jelinek p...@2ndquadrant.com wrote:
On 02/04/15 21:21, Robert Haas wrote:
On Thu, Apr 2, 2015 at 2:07 PM, Robert Haas robertmh...@gmail.com wrote:
I think this is really nice work, so I have committed this version. I
made a few fairly minor changes, hopefully without breaking anything
in the process:
- I adjusted things for recent commits around INT{32,63}_{MIN_MAX}.
- I removed (void) ssup; which I do not think is normal PostgreSQL style.
- Removed the #if DEC_DIGITS != 4 check. The comment is great, but I
think we don't need protect against #if 0 code get re-enabled.
- I removed the too-clever (at least IMHO) handing of TRACE_SORT in
favor of just using #ifdef around each occurrence.
- I also declared trace_sort in guc.h, where various other GUCs are
declared, instead of declaring it privately in each file that needed
it.
- Changed some definitions to depend on SIZEOF_DATUM rather than
USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please check it.
- Fixed an OID conflict.
- And of course, bumped catversion.
And that's broken the buildfarm. Argh.
That's because of this:
+#ifdef SIZEOF_DATUM == 8
You need just #if there.
Thanks. I actually pushed a fix for that about 25 minutes ago;
hopefully that is all that is needed.
--
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] Something is rotten in the state of Denmark...
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 3:59 PM, Tom Lane t...@sss.pgh.pa.us wrote: Perhaps the difference has to do with whether pg_am's pg_class tuple is on a page that hasn't got enough room for a HOT update? But I definitely tried it several times and consistently got the same failure before. That seems plausible, except that I have no idea why that would vary from one test setup to another. I suggest pushing the patch you proposed upthread and seeing what the buildfarm thinks. Yeah. I have errands to do right now but will push it later. 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] extend pgbench expressions with functions
On Sun, Mar 29, 2015 at 2:20 PM, Fabien COELHO coe...@cri.ensmp.fr wrote: Here is a small v2 update: v3, just a rebase. Thanks for working on this. I see it's already registered in the 2015-06 CF, and will have a look at when we get there. -- 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] libpq's multi-threaded SSL callback handling is busted
On 4/2/15 4:32 AM, Jan Urbański wrote: Peter Eisentraut writes: On 2/12/15 7:28 AM, Jan Urbański wrote: For the sake of discussion, here's a patch to prevent stomping on previously-set callbacks, racy as it looks. FWIW, it does fix the Python deadlock and doesn't cause the PHP segfault... I don't think this patch would actually fix the problem that was described after the original bug report (http://www.postgresql.org/message-id/5436991b.5020...@vmware.com), namely that another thread acquires a lock while the libpq callbacks are set and then cannot release the lock if libpq has been shut down in the meantime. I did test both the Python and the PHP repro scripts and the patch fixed both the deadlock and the segfault. What happens is that Python (for instance) stops over the callback unconditionally. So when libpq gets unloaded, it sees that the currently set callback is no the one it originally set and refrains from NULLing it. So this works because Python is just as broken as libpq right now. What happens if Python is fixed as well? Then we'll have the problem I described above. -- 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] Tables cannot have INSTEAD OF triggers
On 4/2/15 11:50 AM, Dean Rasheed wrote: Well actually the fact that the code is structured that way is somewhat academic. INSTEAD OF triggers on views don't support WHEN conditions -- deliberately so, since it would be difficult to know in general what to do if the trigger didn't fire. So ExecInsert is implicitly using the existence of the trigger to imply that it will fire, although arguably it would be neater for it to double-check that, and error out if for some reason the trigger didn't fire. In any case, that doesn't establish any kind of behavioural precedent for how a conditional INSTEAD OF trigger on a table ought to work. I think the upshot is that INSTEAD OF triggers work in a particular way because that's what is needed to support updatable views. If triggers on tables should behave differently, maybe it should be a separate trigger type. Maybe it would be feasible to extend BEFORE triggers to support RETURNING, for example? -- 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] Tables cannot have INSTEAD OF triggers
On 2015-04-02 16:42:43 -0400, Peter Eisentraut wrote: On 4/2/15 11:50 AM, Dean Rasheed wrote: Well actually the fact that the code is structured that way is somewhat academic. INSTEAD OF triggers on views don't support WHEN conditions -- deliberately so, since it would be difficult to know in general what to do if the trigger didn't fire. So ExecInsert is implicitly using the existence of the trigger to imply that it will fire, although arguably it would be neater for it to double-check that, and error out if for some reason the trigger didn't fire. In any case, that doesn't establish any kind of behavioural precedent for how a conditional INSTEAD OF trigger on a table ought to work. I think the upshot is that INSTEAD OF triggers work in a particular way because that's what is needed to support updatable views. If triggers on tables should behave differently, maybe it should be a separate trigger type. Maybe it would be feasible to extend BEFORE triggers to support RETURNING, for example? What in the above prohibits extending the behaviour to tables? I have yet to see what compatibility or similarity problem that'd pose. It seems all mightily handwavy to me. Greetings, Andres Freund -- 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] Something is rotten in the state of Denmark...
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 2:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Actually, now that I think it through, the could not open relation error is pretty odd in itself. If we are trying to open pg_am using a stale catalog snapshot, it seems like we ought to reliably find its old pg_class tuple (the one with the obsolete relfilenode), rather than finding nothing. But the latter is the behavior I'm seeing. What's to stop the old tuple from being HOT-pruned? Hm, that may be it. I went back to the previous test scenario, and now I can *only* get the cache lookup failed for access method behavior, instead of what I was getting before, so I'm getting a bit confused :-(. However, it does seem clear that the mechanism is indeed that we're relying on an obsolete copy of pg_am's pg_class tuple, hence scanning a truncated relfilenode, and that the patch I proposed fixes it. Perhaps the difference has to do with whether pg_am's pg_class tuple is on a page that hasn't got enough room for a HOT update? But I definitely tried it several times and consistently got the same failure before. 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] What exactly is our CRC algorithm?
At 2015-04-03 00:33:10 +0300, hlinn...@iki.fi wrote: I came up with the attached. I like it very much. src/port/Makefile has (note src/srv): +# pg_crc32c_sse42.o and its _src.o version need CFLAGS_SSE42 +pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_SSE42) +pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_SSE42) Other than that, this looks great. Thank you. BTW, we might want to move the traditional and legacy crc32 implementations out of src/common. They are only used in backend code now. I agree. -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Unused variable in hashpage.c
Hi,
my compiler complains about unused variable nblkno in _hash_splitbucket
in no-assert build. It looks like relic of commit ed9cc2b5d which
removed the only use of that variable besides the Assert.
Looking at the code:
nblkno = start_nblkno;
Assert(nblkno == BufferGetBlockNumber(nbuf));
I was originally thinking of simply changing the Assert to use
start_nblkno but then I noticed that the start_nblkno is actually not
used anywhere in the function either (it's one of input parameters). And
given that the only caller of that function is getting the variable it
sends as nbuf to that function via start_nblkno, I think the Assert is
somewhat pointless there anyway.
So best way to solve this seems to be removing the start_nblkno from the
_hash_splitbucket altogether. Attached patch does just that.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 9a77945..2f82b1e 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -39,7 +39,6 @@ static bool _hash_alloc_buckets(Relation rel, BlockNumber firstblock,
static void _hash_splitbucket(Relation rel, Buffer metabuf,
Buffer nbuf,
Bucket obucket, Bucket nbucket,
- BlockNumber start_oblkno,
BlockNumber start_nblkno,
uint32 maxbucket,
uint32 highmask, uint32 lowmask);
@@ -666,8 +665,7 @@ _hash_expandtable(Relation rel, Buffer metabuf)
/* Relocate records to the new bucket */
_hash_splitbucket(rel, metabuf, buf_nblkno,
old_bucket, new_bucket,
- start_oblkno, start_nblkno,
- maxbucket, highmask, lowmask);
+ start_oblkno, maxbucket, highmask, lowmask);
/* Release bucket locks, allowing others to access them */
_hash_droplock(rel, start_oblkno, HASH_EXCLUSIVE);
@@ -758,13 +756,11 @@ _hash_splitbucket(Relation rel,
Bucket obucket,
Bucket nbucket,
BlockNumber start_oblkno,
- BlockNumber start_nblkno,
uint32 maxbucket,
uint32 highmask,
uint32 lowmask)
{
BlockNumber oblkno;
- BlockNumber nblkno;
Buffer obuf;
Page opage;
Page npage;
@@ -781,8 +777,6 @@ _hash_splitbucket(Relation rel,
opage = BufferGetPage(obuf);
oopaque = (HashPageOpaque) PageGetSpecialPointer(opage);
- nblkno = start_nblkno;
- Assert(nblkno == BufferGetBlockNumber(nbuf));
npage = BufferGetPage(nbuf);
/* initialize the new bucket's primary page */
--
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] Table-level log_autovacuum_min_duration
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -881,9 +881,11 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } |
UNLOGGED ] TABLE [ IF NOT EXI
literaltoast./literal, which can be used to control the behavior of the
table's secondary acronymTOAST/ table, if any
(see xref linkend=storage-toast for more information about TOAST).
-Note that the TOAST table inherits the
-literalautovacuum_*/literal values from its parent table, if there are
-no literaltoast.autovacuum_*/literal settings set.
+Note that the TOAST table inherits values of
literalautovacuum_*/literal
+and literallog_autovacuum_min_duration/literal from its parent table,
if
+there are no values set for respectively
+literaltoast.autovacuum_*/literal and
+literaltoast.log_autovacuum_min_duration/literal.
/para
I think this could use some wordsmithing; I didn't like listing the
parameters explicitely, and Jaime Casanova suggested not using the terms
inherit and parent table in this context. So I came up with this:
Note that the TOAST table uses the parameter values defined for the main
table, for each parameter applicable to TOAST tables and for which no
value is set in the TOAST table itself.
Thoughts?
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Abbreviated keys for Datum tuplesort
On Thu, Apr 2, 2015 at 11:21 PM, Robert Haas robertmh...@gmail.com wrote: The changes that Andrew took issue with are utterly insignificant. Great. Then you will be utterly indifferent to which version gets committed. *shrug* You were the one that taught me to be bureaucratically minded about keeping code consistent at this fine a level. I think it's odd that you of all people are opposing me on this point, but whatever. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Abbreviated keys for Numeric
Robert == Robert Haas robertmh...@gmail.com writes:
Robert I think this is really nice work, so I have committed this
Robert version. I made a few fairly minor changes, hopefully without
Robert breaking anything in the process:
Robert - I adjusted things for recent commits around INT{32,63}_{MIN_MAX}.
Robert - I removed (void) ssup; which I do not think is normal PostgreSQL
style.
Robert - Removed the #if DEC_DIGITS != 4 check. The comment is great, but I
Robert think we don't need protect against #if 0 code get re-enabled.
No complaints in principle for any of these
Robert - I removed the too-clever (at least IMHO) handing of TRACE_SORT in
Robert favor of just using #ifdef around each occurrence.
My idea here - which I have admittedly not got round to posting a patch
for yet - was that TRACE_SORT (which has been defined by default since
8.1) should be deprecated in favour of unconditional use of trace_sort.
(The actual definition of TRACE_SORT could be kept in case any extension
or whatever uses it)
Robert - I also declared trace_sort in guc.h, where various other GUCs
Robert are declared, instead of declaring it privately in each file
Robert that needed it.
I was thinking miscadmin.h but whatever.
Robert - Changed some definitions to depend on SIZEOF_DATUM rather
Robert than USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please
Robert check it.
No, that's wrong; it must use USE_FLOAT8_BYVAL since that's what the
definitions of Int64GetDatum / DatumGetInt64 are conditional on. As you
have it now, it'll break if you build with --disable-float8-byval on a
64bit platform.
--
Andrew (irc:RhodiumToad)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Abbreviated keys for Datum tuplesort
On Thu, Apr 2, 2015 at 11:21 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 2, 2015 at 5:34 PM, Peter Geoghegan p...@heroku.com wrote: I think it's explained by the pre-check for sorted input making the number of comparisons exactly n -1. As I pointed out to Tomas, if you put a single, solitary unsorted element at the end, the abbreviated version is then 8x faster (maybe that was in relation to a slightly different case, but the pattern is the same). So this case isn't an argument against datum abbreviation, or even abbreviation in general, but rather an argument against using strxfrm() in general (which for example the GCC docs strongly recommend for sorting lists of strings). It's a bad argument, IMV. This sort is already extremely fast. OK, I see. Wait. It's also due to the fact that some cases are spuriously aborted, because the cost model for text needs to be fixed, I believe. It's not clear that this is actually such a case from Tomas' remarks, because the smaller scale tested *did* win significantly, and that appeared to be otherwise comparable to the regressed cases. So although what I describe here about perfectly pre-sorted input is something I've seen (and something that we discussed on another thread IIRC), this is not an example of it. Again, this is just an example of why we need to fix the cost model for text. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSoC 2015 proposal: Support for microvacuum for GiST
On 03/26/2015 11:37 PM, Ilia Ivanicki wrote: *Abstract:* Currently support for microvacuum is implemented only for BTree index. But GiST index is so useful and widely used for user defined datatypes instead of btree. During index search it reads page by page. Every tuple on the page in buffer marked as dead if it doesn't visible for all transactions. Whenever before receiving next page we check dead items and mark current page as has garbage[1]. When the page gets full, all the killed items are removed by calling microvacuum[2]. Seems reasonable. Should be a pretty straightforward to implement. *Project Schedule * until May 31 Solve architecture questions with help of community. 1 June – 30 June First, approximate implementation supporting microvacuum for GiST. I’ve got bachelor's degree in this month so I haven’t much time to work on project. 1 July – 31 July Implementation of supporting microvacuum for GiST and testing. 1 August -15 August Final refactoring, testing and committing. GSoC should be treated as a full-time job, that's how much time you're expected to dedicate to it. Having bachelor's degree exams in June would be a serious problem. You'll need to discuss with the potential mentors on how to make up for that time. Other than that, the schedule seems fairly relaxed. In fact, this project seems a bit too small for a GSoC project. I'd suggest coming up with some additional GiST-related work that you could do, in addition to the microvacuum thing. Otherwise I think there's a risk that you finish the patch in May, and have nothing to do for the rest of the summer. - 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] Additional role attributes superuser review
On Thu, Apr 2, 2015 at 12:53 AM, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: REVOKE'ing access *without* removing the permissions checks would defeat the intent of these changes, which is to allow an administrator to grant the ability for a certain set of users to cancel and/or terminate backends started by other users, without also granting those users superuser rights. I see: we have two different use-cases and no way for GRANT/REVOKE to manage both cases using permissions on a single object. Carry on then. Alright, after going about this three or four different ways, I've settled on the approach proposed in the attached patch. Most of it is pretty straight-forward: drop the hard-coded check in the function itself and REVOKE EXECUTE on the function from PUBLIC. The interesting bits are those pieces which are more complex than the all-or-nothing cases. This adds a few new SQL-level functions which return unfiltered results, if you're allowed to call them based on the GRANT system (and EXECUTE privileges for them are REVOKE'd from PUBLIC, of course). These are: pg_stat_get_activity_all, pg_stat_get_wal_senders_all, and pg_signal_backend (which is similar but not the same- that allows you to send signals to other backends as a regular user, if you are allowed to call that function and the other backend wasn't started by a superuser). There are two new views added, which simply sit on top of the new functions: pg_stat_activity_all and pg_stat_replication_all. Minimizing the amount of code duplication wasn't too hard with the pg_stat_get_wal_senders case; as it was already returning a tuplestore and so I simply moved most of the logic into a helper function which handled populating the tuplestore and then each call path was relatively small and mostly boilerplate. pg_stat_get_activity required a bit more in the way of changes, but they were essentially to modify it to return a tuplestore like pg_stat_get_wal_senders, and then add in the extra function and boilerplate for the non-filtered path. I had considered (and spent a good bit of time implementing...) a number of alternatives, mostly around trying to do more at the SQL level when it came to filtering, but that got very ugly very quickly. There's no simple way to find out what was the effective role of the caller of this function from inside of a security definer function (which would be required for the filtering, as it would need to be able to call the function underneath). This is necessary, of course, because functions in views run as the caller of the view, not as the view owner (that's useful in some cases, less useful in others). I looked at exposing the information about the effective role which was calling a function, but that got pretty ugly too. The GUC system has a stack, but we don't actually update the 'role' GUC when we are in security definer functions. There's the notion of an outer role ID, but that doesn't account for intermediate security definer functions and so, while it's fine for what it does, it wasn't really helpful in this case. I do still think it'd be nice to provide that information and perhaps we can do so with fmgr_security_definer(), but it's beyond what's really needed here and I don't think it'd end up being particularly cleaner to do it all in SQL now that I've refactored pg_stat_get_activity. I'd further like to see the ability to declare on either a function call by function call basis (inside a view defintion), of even at the view level (as that would allow me to build up multiple views with different parameters) who to run this function as, where the options would be view owner or view querier, very similar to our SECURITY DEFINER vs. SECURITY INVOKER options for functions today. This is interesting because, more and more, we're building functions which are actually returning data sets, not individual values, and we want to filter those sets sometimes and not other times. In any case, those are really thoughts for the future and get away from what this is all about, which is reducing the need for monitoring and/or regular admins to have the superuser bit when they don't really need it. Clearly, further testing and documentation is required and I'll be getting to that over the next couple of days, but it's pretty darn late and I'm currently getting libpq undefined reference errors, probably because I need to set LD_LIBRARY_PATH, but I'm just too tired to now. :) Thoughts? The tricky part of this seems to me to be the pg_dump changes. The new catalog flag seems a little sketchy to me; wouldn't it be better to make the dump flag into an enum, DUMP_EVERYTHING, DUMP_ACL, DUMP_NONE? Is this creating a user-visible API break, where pg_stat_activity and pg_stat_replication now always show only the publicly-visible information, and privileged users
Re: [HACKERS] Sloppy SSPI error reporting code
On Thu, Apr 2, 2015 at 01:44:59AM -0400, Noah Misch wrote: On Wed, Apr 01, 2015 at 10:49:01PM -0400, Bruce Momjian wrote: On Sat, Jan 10, 2015 at 02:53:13PM -0500, Tom Lane wrote: While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. I looked through that file and only found two cases; patch attached. Tom mentioned newline omissions, which you'll find in pg_SSPI_error(). Oh, I accidentally saw the backend version of that function, which looked fine. I have attached a patch for that. ! printfPQExpBuffer(conn-errorMessage, libpq_gettext(SSPI returned invalid number of output buffers\n)); !libpq_gettext(fe_sendauth: error sending password authentication\n)); The first message is too internals-focused for a translation to be useful. If it were in the backend, we would use elog() and not translate. The second is a non-actionable message painting over a wide range of specific failures; translating it would just put lipstick on the pig. OK. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c new file mode 100644 index 8927df4..08cc906 *** a/src/interfaces/libpq/fe-auth.c --- b/src/interfaces/libpq/fe-auth.c *** pg_SSPI_error(PGconn *conn, const char * *** 236,245 if (FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, r, 0, sysmsg, sizeof(sysmsg), NULL) == 0) ! printfPQExpBuffer(conn-errorMessage, %s: SSPI error %x, mprefix, (unsigned int) r); else ! printfPQExpBuffer(conn-errorMessage, %s: %s (%x), mprefix, sysmsg, (unsigned int) r); } --- 236,245 if (FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, r, 0, sysmsg, sizeof(sysmsg), NULL) == 0) ! printfPQExpBuffer(conn-errorMessage, %s: SSPI error %x\n, mprefix, (unsigned int) r); else ! printfPQExpBuffer(conn-errorMessage, %s: %s (%x)\n, mprefix, sysmsg, (unsigned int) r); } -- 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] Something is rotten in the state of Denmark...
On Wed, Apr 1, 2015 at 7:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: I've been able to reproduce this. The triggering event seems to be that the VACUUM FULL pg_am in vacuum.sql has to happen while another backend is starting up. With a ten-second delay inserted at the bottom of PerformAuthentication(), it's trivial to hit it manually. The reason we'd not seen this before the rolenames.sql test was added is that none of the other tests that run concurrently with vacuum.sql perform mid-test reconnections, or ever have AFAIR. So as long as they all managed to start up before vacuum.sql got to the dangerous step, no problem. I've not fully tracked it down, but I think that the blame falls on the MVCC-snapshots-for-catalog-scans patch; it appears that it's trying to read pg_am's pg_class entry with a snapshot that's too old, possibly because it assumes that sinval signaling is alive which I think ain't so. Hmm, sorry for the bug. It looks to me like sinval signaling gets started up at the beginning of InitPostgres(). Perhaps something like this? diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 7cfa0cf..47c4002 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -303,7 +303,7 @@ GetNonHistoricCatalogSnapshot(Oid relid) * Mark new snapshost as valid. We must do this last, in case an * ERROR occurs inside GetSnapshotData(). */ - CatalogSnapshotStale = false; + CatalogSnapshotStale = !IsNormalProcessingMode(); } return CatalogSnapshot; -- 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] Assertion failure when streaming logical changes
On 10 February 2015 at 21:43, Andres Freund and...@2ndquadrant.com wrote: On 2015-02-10 22:06:34 +0900, Michael Paquier wrote: On Tue, Feb 10, 2015 at 9:46 PM, Andres Freund and...@2ndquadrant.com wrote: Yea, it really looks like the above commit is to blame. The new xmin tracking infrastructure doesn't know about the historical snapshot... I think that we need a better regression coverage here... For example, we could add some tap tests in test_decoding to test streaming of logical changes. This would help in the future to detect such problems via the buildfarm. I agree. It's more or less a accident that the assert - which just should be moved in the regd_count == 0 branch - didn't trigger for the SQL interface. The snapshot acquired by the SELECT statement prevents it there. It's not entirely trivial to add tests for receivelogical though. You need to stop it programatically after a while. I reckon we should improve that then. What about an idle timeout for pg_recvlogical, so we can get it to time out after (say) 5 seconds of no data? Time-based facilities aren't great in tests though. The SQL interface can stop after a given number of changes received or a target LSN. Is there a practical reason pg_recvlogical doesn't? Or just that there wasn't a reason to implement it? I figured I'd ask before jumping in and trying to add it in case I'd be wasting my time trying. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] The return value of allocate_recordbuf()
On Fri, Apr 3, 2015 at 12:56 PM, Fujii Masao wrote:
The first patch looks good to me basically. But I have one comment:
shouldn't we expose pg_malloc_extended as a global function like
we did pg_malloc? Some frontends might need to use it in the future.
Yes, it makes sense as the other functions do it too. So I refactored
the patch and defined a new static inline routine,
pg_malloc_internal(), that all the other functions call to reduce the
temperature in this code path that I suppose can become quite hot even
for frontends. In the second patch I added as well what was needed for
pg_rewind.
--
Michael
From f43fc9a5a5e0ffb246782010b8860829b8304593 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Fri, 3 Apr 2015 14:21:12 +0900
Subject: [PATCH 1/2] Add palloc_extended and pg_malloc_extended for frontend
and backend
This interface can be used to control at a lower level memory allocation
using an interface similar to MemoryContextAllocExtended, but this time
applied to CurrentMemoryContext on backend side, while on frontend side
a similar interface is available.
---
src/backend/utils/mmgr/mcxt.c| 37 ++
src/common/fe_memutils.c | 43 ++--
src/include/common/fe_memutils.h | 11 ++
src/include/utils/palloc.h | 1 +
4 files changed, 81 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index e2fbfd4..c42a6b6 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -864,6 +864,43 @@ palloc0(Size size)
return ret;
}
+void *
+palloc_extended(Size size, int flags)
+{
+ /* duplicates MemoryContextAllocExtended to avoid increased overhead */
+ void *ret;
+
+ AssertArg(MemoryContextIsValid(CurrentMemoryContext));
+ AssertNotInCriticalSection(CurrentMemoryContext);
+
+ if (((flags MCXT_ALLOC_HUGE) != 0 !AllocHugeSizeIsValid(size)) ||
+ ((flags MCXT_ALLOC_HUGE) == 0 !AllocSizeIsValid(size)))
+ elog(ERROR, invalid memory alloc request size %zu, size);
+
+ CurrentMemoryContext-isReset = false;
+
+ ret = (*CurrentMemoryContext-methods-alloc) (CurrentMemoryContext, size);
+ if (ret == NULL)
+ {
+ if ((flags MCXT_ALLOC_NO_OOM) == 0)
+ {
+ MemoryContextStats(TopMemoryContext);
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg(out of memory),
+ errdetail(Failed on request of size %zu., size)));
+ }
+ return NULL;
+ }
+
+ VALGRIND_MEMPOOL_ALLOC(CurrentMemoryContext, ret, size);
+
+ if ((flags MCXT_ALLOC_ZERO) != 0)
+ MemSetAligned(ret, 0, size);
+
+ return ret;
+}
+
/*
* pfree
* Release an allocated chunk.
diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c
index 345221e..527f9c8 100644
--- a/src/common/fe_memutils.c
+++ b/src/common/fe_memutils.c
@@ -19,8 +19,8 @@
#include postgres_fe.h
-void *
-pg_malloc(size_t size)
+static inline void *
+pg_malloc_internal(size_t size, int flags)
{
void *tmp;
@@ -28,22 +28,37 @@ pg_malloc(size_t size)
if (size == 0)
size = 1;
tmp = malloc(size);
- if (!tmp)
+ if (tmp == NULL)
{
- fprintf(stderr, _(out of memory\n));
- exit(EXIT_FAILURE);
+ if ((flags MCXT_ALLOC_NO_OOM) == 0)
+ {
+ fprintf(stderr, _(out of memory\n));
+ exit(EXIT_FAILURE);
+ }
+ return NULL;
}
+
+ if ((flags MCXT_ALLOC_ZERO) != 0)
+ MemSet(tmp, 0, size);
return tmp;
}
void *
+pg_malloc(size_t size)
+{
+ return pg_malloc_internal(size, 0);
+}
+
+void *
pg_malloc0(size_t size)
{
- void *tmp;
+ return pg_malloc_internal(size, MCXT_ALLOC_ZERO);
+}
- tmp = pg_malloc(size);
- MemSet(tmp, 0, size);
- return tmp;
+void *
+pg_malloc_extended(size_t size, int flags)
+{
+ return pg_malloc_internal(size, flags);
}
void *
@@ -100,13 +115,19 @@ pg_free(void *ptr)
void *
palloc(Size size)
{
- return pg_malloc(size);
+ return pg_malloc_internal(size, 0);
}
void *
palloc0(Size size)
{
- return pg_malloc0(size);
+ return pg_malloc_internal(size, MCXT_ALLOC_ZERO);
+}
+
+void *
+palloc_extended(Size size, int flags)
+{
+ return pg_malloc_internal(size, flags);
}
void
diff --git a/src/include/common/fe_memutils.h b/src/include/common/fe_memutils.h
index db7cb3e..6466167 100644
--- a/src/include/common/fe_memutils.h
+++ b/src/include/common/fe_memutils.h
@@ -9,10 +9,20 @@
#ifndef FE_MEMUTILS_H
#define FE_MEMUTILS_H
+/*
+ * Flags for pg_malloc_extended and palloc_extended, deliberately named
+ * the same as the backend flags.
+ */
+#define MCXT_ALLOC_HUGE 0x01 /* allow huge allocation ( 1 GB)
+ * not actually used for frontends */
+#define MCXT_ALLOC_NO_OOM 0x02 /* no failure if out-of-memory */
+#define MCXT_ALLOC_ZERO 0x04 /* zero allocated memory */
+
/* Safe memory allocation functions --- these exit(1) on failure */
extern char *pg_strdup(const char *in);
extern void *pg_malloc(size_t size);
extern void *pg_malloc0(size_t size);
+extern void *pg_malloc_extended(size_t size, int
Re: [HACKERS] The return value of allocate_recordbuf()
On Thu, Feb 12, 2015 at 4:02 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Feb 11, 2015 at 2:13 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:02 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Mon, Feb 9, 2015 at 7:58 PM, Fujii Masao masao.fu...@gmail.com wrote: MemoryContextAllocExtended() was added, so isn't it time to replace palloc() with MemoryContextAllocExtended(CurrentMemoryContext, MCXT_ALLOC_NO_OOM) in allocate_recordbuf()? Yeah, let's move on here, but not with this patch I am afraid as this breaks any client utility using xlogreader.c in frontend, like pg_xlogdump or pg_rewind because MemoryContextAllocExtended is not available in frontend, only in backend. We are going to need something like palloc_noerror, defined on both backend (mcxt.c) and frontend (fe_memutils.c) side. Unfortunately, that gets us back into the exact terminological dispute that we were hoping to avoid. Perhaps we could compromise on palloc_extended(). Yes, why not using palloc_extended instead of palloc_noerror that has been clearly rejected in the other thread. Now, for palloc_extended we should copy the flags of MemoryContextAllocExtended to fe_memutils.h and have the same interface between frontend and backend (note that MCXT_ALLOC_HUGE has no real utility for frontends). Attached is a patch to achieve that, completed with a second patch for the problem related to this thread. Note that in the second patch I have added an ERROR in logical.c after calling XLogReaderAllocate, this was missing.. The first patch looks good to me basically. But I have one comment: shouldn't we expose pg_malloc_extended as a global function like we did pg_malloc? Some frontends might need to use it in the future. Regards, -- Fujii Masao -- 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] Something is rotten in the state of Denmark...
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 2:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: However, I'm having second thoughts about whether we've fully diagnosed this. Three out of the four failures we've seen in the buildfarm reported cache lookup failed for access method 403, not could not open relation with OID 2601 ... and I'm so far only able to replicate the latter symptom. It's really unclear how the former one could arise, because nothing that vacuum.sql does would change xmin of the rows in pg_am. It probably changes the *relfilenode* of pg_am, because it runs VACUUM FULL on that catalog. Perhaps some backend sees the old relfilenode value and tries to a heap scan, interpreting the now-truncated file as empty? Yeah, I came up with the same theory a few minutes later. Trying to reproduce on that basis. Actually, now that I think it through, the could not open relation error is pretty odd in itself. If we are trying to open pg_am using a stale catalog snapshot, it seems like we ought to reliably find its old pg_class tuple (the one with the obsolete relfilenode), rather than finding nothing. But the latter is the behavior I'm seeing. 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] Re: Abbreviated keys for Datum tuplesort
On Fri, Feb 20, 2015 at 3:57 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: I've spent a fair amount of testing this today, and when using the simple percentile_disc example mentioned above, I see this pattern: master patched speedup - generate_series(1,100) 4.2 0.7 6 generate_series(1,200) 9.2 9.8 0.93 generate_series(1,300) 14.5 15.3 0.95 so for a small dataset the speedup is very nice, but for larger sets there's ~5% slowdown. Is this expected? I had a look at this patch today with a view to committing it, but it seems that nobody's commented on this point, which seems like an important one. Any thoughts? For what it's worth, and without wishing to provoke another flamewar, I am inclined to use Andrew's version of this patch rather than Peter's. I have not undertaken an exhaustive comparison, nor do I intend to. It is the reviewer's responsibility to justify the changes they think the author needs to make, and that wasn't done here. On the points of difference Andrew highlighted, I think his version is fine. -- 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] Something is rotten in the state of Denmark...
On Thu, Apr 2, 2015 at 2:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 2:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: However, I'm having second thoughts about whether we've fully diagnosed this. Three out of the four failures we've seen in the buildfarm reported cache lookup failed for access method 403, not could not open relation with OID 2601 ... and I'm so far only able to replicate the latter symptom. It's really unclear how the former one could arise, because nothing that vacuum.sql does would change xmin of the rows in pg_am. It probably changes the *relfilenode* of pg_am, because it runs VACUUM FULL on that catalog. Perhaps some backend sees the old relfilenode value and tries to a heap scan, interpreting the now-truncated file as empty? Yeah, I came up with the same theory a few minutes later. Trying to reproduce on that basis. Actually, now that I think it through, the could not open relation error is pretty odd in itself. If we are trying to open pg_am using a stale catalog snapshot, it seems like we ought to reliably find its old pg_class tuple (the one with the obsolete relfilenode), rather than finding nothing. But the latter is the behavior I'm seeing. What's to stop the old tuple from being HOT-pruned? -- 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] Abbreviated keys for Numeric
On Thu, Apr 2, 2015 at 2:07 PM, Robert Haas robertmh...@gmail.com wrote:
I think this is really nice work, so I have committed this version. I
made a few fairly minor changes, hopefully without breaking anything
in the process:
- I adjusted things for recent commits around INT{32,63}_{MIN_MAX}.
- I removed (void) ssup; which I do not think is normal PostgreSQL style.
- Removed the #if DEC_DIGITS != 4 check. The comment is great, but I
think we don't need protect against #if 0 code get re-enabled.
- I removed the too-clever (at least IMHO) handing of TRACE_SORT in
favor of just using #ifdef around each occurrence.
- I also declared trace_sort in guc.h, where various other GUCs are
declared, instead of declaring it privately in each file that needed
it.
- Changed some definitions to depend on SIZEOF_DATUM rather than
USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please check it.
- Fixed an OID conflict.
- And of course, bumped catversion.
And that's broken the buildfarm. Argh.
--
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] What exactly is our CRC algorithm?
On 04/02/2015 12:39 PM, Abhijit Menon-Sen wrote: At 2015-03-25 19:18:51 +0200, hlinn...@iki.fi wrote: I think we'll need a version check there. […] You want to write that or should I? I'm not familiar with MSVC at all, so it would be nice if you did it. Thinking more about the configure checks, I think the current approach of using inline assembly on gcc is not quite right. We're only using inline assembly to force producing SSE 4.2 code, even when -msse4.2 is not used. That feels wrong. And who's to say that the assembler supports the SSE instructions anyway? At least we'd need a configure check for that too. We have a buildfarm animal that still uses gcc 2.95.3, which was released in 2001. I don't have a compiler of that vintage to test with, but I assume an old enough assembler would not know about the crc32q instruction and fail to compile. I believe the GCC way to do this would be to put the SSE4.2-specific code into a separate source file, and compile that file with -msse4.2. And when you compile with -msse4.2, gcc actually also supports the _mm_crc32_u8/u64 intrinsics. - 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] Tables cannot have INSTEAD OF triggers
On 2 April 2015 at 14:59, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Apr 1, 2015 at 1:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: It would absolutely *not* be reasonable for WHEN conditions for triggers on tables to work completely differently than they do for triggers on views. That ship's sailed. Clue me in, because I'm confused. If no trigger fires, we do whatever an object of that type would normally do in the absence of any trigger, no? For a view, that's error out; for a table, that's perform the action on the underlying data. That doesn't seem terribly unprincipled. I dunno about unprincipled; but we have already laid down the definition of INSTEAD OF triggers, and they act as I described. Read the code if you doubt it: which path is taken in ExecInsert depends only on whether INSTEAD OF triggers *exist* on the rel, not whether any of them actually fired (indeed, it would be difficult even to know that from here). I believe this was intentional, not just a coding artifact; it stems from having wanted to throw the error for uninsertable view well upstream of here, rather than having it be conditional on what happens at runtime. What I am objecting to is Andres' claim that it would be okay for INSTEAD OF triggers on tables to act completely differently in this regard from those on views. We have laid down the definition for views, and it is that nothing happens if the trigger exists but doesn't fire. Well actually the fact that the code is structured that way is somewhat academic. INSTEAD OF triggers on views don't support WHEN conditions -- deliberately so, since it would be difficult to know in general what to do if the trigger didn't fire. So ExecInsert is implicitly using the existence of the trigger to imply that it will fire, although arguably it would be neater for it to double-check that, and error out if for some reason the trigger didn't fire. In any case, that doesn't establish any kind of behavioural precedent for how a conditional INSTEAD OF trigger on a table ought to work. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Centralize definition of integer limits.
On 2015-04-02 10:42:58 -0400, Robert Haas wrote: On Tue, Mar 31, 2015 at 12:15 PM, Andres Freund and...@anarazel.de wrote: I'm tempted to just prefix our limits with PG_ and define them unconditionally, including appropriate casts to our types. I don't have a better idea. Will push that. I'd appreciate it if you could do this soon. I like to compile with -Werror, and this problem means I can't. Done. Sorry for not doing this sooner, I'm more or less having holidays right now. Greetings, Andres Freund -- 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] What exactly is our CRC algorithm?
On 2015-04-02 20:57:24 +0530, Abhijit Menon-Sen wrote: At 2015-04-02 17:58:23 +0300, hlinn...@iki.fi wrote: We're only using inline assembly to force producing SSE 4.2 code, even when -msse4.2 is not used. That feels wrong. Why? It feels OK to me (and to Andres, per earlier discussions about exactly this topic). Doing it this way allows the binary to run on a non-SSE4.2 platform (and not use the CRC instructions). Right. And SSE4.2 isn't that widespread yet. I believe the GCC way to do this would be to put the SSE4.2-specific code into a separate source file, and compile that file with -msse4.2. And when you compile with -msse4.2, gcc actually also supports the _mm_crc32_u8/u64 intrinsics. To me this seems like a somewhat pointless exercise. I actually think from a performance POV it's better to have all the functions in one source file, so the compiler can inline things into the trampoline if it feels like it. Greetings, Andres Freund -- 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] deparsing utility commands
On Wed, Mar 25, 2015 at 3:21 PM, Ryan Pedela rped...@datalanche.com wrote: On Wed, Mar 25, 2015 at 11:59 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: * Should we prohibit DDL from within event triggers? Please don't prohibit DDL unless there is a really, really good reason to do so. I have several use cases in mind for event triggers, but they are only useful if I can perform DDL. For example, I want to create an Elasticsearch FDW and use that to index and search Postgres tables. When a table is created, I am planning to use an event trigger to capture the CREATE event and automatically create a foreign table via the Elasticsearch FDW. In addition, I would add normal triggers to the new table which capture DML and update the foreign table accordingly. In other words, I want to use FDWs and event triggers to automatically sync table DDL and DML to Elasticsearch. +1. I think that if it's unsafe to run DDL from with event triggers, then that might be a sign that it's not safe to run *any* user-defined code at that location. I think it's the job of the event trigger to make sure that it doesn't assume anything about what may have changed while the user-defined code was running. -- 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] What exactly is our CRC algorithm?
At 2015-04-02 17:58:23 +0300, hlinn...@iki.fi wrote: We're only using inline assembly to force producing SSE 4.2 code, even when -msse4.2 is not used. That feels wrong. Why? It feels OK to me (and to Andres, per earlier discussions about exactly this topic). Doing it this way allows the binary to run on a non-SSE4.2 platform (and not use the CRC instructions). Also, -msse4.2 was added to the compiler later than support for the instructions was added to the assembler. We have a buildfarm animal that still uses gcc 2.95.3, which was released in 2001. I don't have a compiler of that vintage to test with, but I assume an old enough assembler would not know about the crc32q instruction and fail to compile. GCC from 2002 wouldn't support the symbolic operand names in inline assembly. binutils from 2007 (IIRC) wouldn't support the assembler instructions themselves. We could work around the latter by using the appropriate sequence of bytes. We could work around the former by using the old syntax for operands. I believe the GCC way to do this would be to put the SSE4.2-specific code into a separate source file, and compile that file with -msse4.2. And when you compile with -msse4.2, gcc actually also supports the _mm_crc32_u8/u64 intrinsics. I have no objection to this. Building only that file with -msse4.2 would resolve the problem of the output binary requiring SSE4.2; and the only compilers to be excluded are old enough to be uninteresting at least to me personally. Have you done/are you doing this already, or do you want me to? I could use advice on how to add build flags to only one file, since I don't know of any precendent for that. -- 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] varlena.c hash_any() and hash_uint32() calls require DatumGetUInt32()
On Wed, Mar 25, 2015 at 9:55 AM, Peter Geoghegan p...@heroku.com wrote: Attached patch adds DatumGetUInt32() around the hash_any() and hash_uint32() calls within varlena.c. These should have been in the original abbreviated keys commit. Mea culpa. Committed. Sorry for the delay; I'm still catching up from last week, when I was at PGCONF.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
Re: [HACKERS] [COMMITTERS] pgsql: Centralize definition of integer limits.
On Thu, Apr 2, 2015 at 11:54 AM, Andres Freund and...@anarazel.de wrote: On 2015-04-02 10:42:58 -0400, Robert Haas wrote: On Tue, Mar 31, 2015 at 12:15 PM, Andres Freund and...@anarazel.de wrote: I'm tempted to just prefix our limits with PG_ and define them unconditionally, including appropriate casts to our types. I don't have a better idea. Will push that. I'd appreciate it if you could do this soon. I like to compile with -Werror, and this problem means I can't. Done. Sorry for not doing this sooner, I'm more or less having holidays right now. Thanks. Sorry to interrupt your vacation. -- 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 Seq Scan
On Thu, Apr 2, 2015 at 3:07 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Wed, Apr 1, 2015 at 6:11 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Apr 1, 2015 at 7:30 AM, Amit Kapila amit.kapil...@gmail.com wrote: Also, the new code to propagate XactLastRecEnd won't work right, either. As we are generating FATAL error on termination of worker (bgworker_die()), so won't it be handled in AbortTransaction path by below code in parallel-mode patch? That will asynchronously flush the WAL, but if the master goes on to commit, we've wait synchronously for WAL flush, and possibly sync rep. Okay, so you mean if master backend later commits, then there is a chance of loss of WAL data written by worker. Can't we report the location to master as the patch does in case of Commit in worker? That's exactly why I think it needs to call WaitForParallelWorkersToFinish() - because it will do just that. We only need to invent a way of telling the worker to stop the scan and shut down cleanly. -- 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] Tables cannot have INSTEAD OF triggers
On Wed, Apr 1, 2015 at 1:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@anarazel.de writes: On 2015-04-01 13:29:33 -0400, Tom Lane wrote: WHEN won't help; if there are any INSTEAD OF triggers, no insert will happen, whether the triggers actually fire or not. Well, right now it doesn't work at all. It seems pretty reasonable to define things so that the insert happens normally if there's no matching INSTEAD OF trigger. It would absolutely *not* be reasonable for WHEN conditions for triggers on tables to work completely differently than they do for triggers on views. That ship's sailed. Clue me in, because I'm confused. If no trigger fires, we do whatever an object of that type would normally do in the absence of any trigger, no? For a view, that's error out; for a table, that's perform the action on the underlying data. That doesn't seem terribly unprincipled. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Centralize definition of integer limits.
On Tue, Mar 31, 2015 at 12:15 PM, Andres Freund and...@anarazel.de wrote: I'm tempted to just prefix our limits with PG_ and define them unconditionally, including appropriate casts to our types. I don't have a better idea. Will push that. I'd appreciate it if you could do this soon. I like to compile with -Werror, and this problem means I can't. -- 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 Seq Scan
On Thu, Apr 2, 2015 at 2:36 AM, Amit Kapila amit.kapil...@gmail.com wrote: If I'm not confused, it would be the other way around. We would run the initPlan in the master backend *first* and then the rest in the workers. Either one of us is confused, let me try to describe my understanding in somewhat more detail. Let me try to explain w.r.t the tab completion query [1]. In this, the initPlan is generated for Qualification expression [2], so it will be executed during qualification and the callstack will look like: postgres.exe!ExecSeqScan(ScanState * node=0x0c33bce8) Line 113 C postgres.exe!ExecProcNode(PlanState * node=0x0c33bce8) Line 418 + 0xa bytes C postgres.exe!ExecSetParamPlan(SubPlanState * node=0x0c343930, ExprContext * econtext=0x0c33de50) Line 1001 + 0xa bytes C postgres.exe!ExecEvalParamExec(ExprState * exprstate=0x0c33f980, ExprContext * econtext=0x0c33de50, char * isNull=0x0c33f481, ExprDoneCond * isDone=0x) Line C postgres.exe!ExecMakeFunctionResultNoSets(FuncExprState * fcache=0x0c33f0d0, ExprContext * econtext=0x0c33de50, char * isNull=0x0042f1c8, ExprDoneCond * isDone=0x) Line 1992 + 0x2d bytes C postgres.exe!ExecEvalOper(FuncExprState * fcache=0x0c33f0d0, ExprContext * econtext=0x0c33de50, char * isNull=0x0042f1c8, ExprDoneCond * isDone=0x) Line 2443 C postgres.exe!ExecQual(List * qual=0x0c33fa08, ExprContext * econtext=0x0c33de50, char resultForNull=0) Line 5206 + 0x1a bytes C postgres.exe!ExecScan(ScanState * node=0x0c33dd38, TupleTableSlot * (ScanState *)* accessMtd=0x000140232940, char (ScanState *, TupleTableSlot *)* recheckMtd=0x0001402329e0) Line 195 + 0x1a bytes C postgres.exe!ExecSeqScan(ScanState * node=0x0c33dd38) Line 114 C Basically here initPlan is getting executed during Qualification. OK, I failed to realize that the initPlan doesn't get evaluated until first use. Maybe in the case of a funnel node we should force all of the initplans to be run before starting parallelism, so that we can pass down the resulting value to each worker. If we try to push the whole plan tree down from the worker then, aside from the issue of needing to copy the plan tree, it'll get evaluated N times instead of once. -- 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] Tables cannot have INSTEAD OF triggers
Robert Haas robertmh...@gmail.com writes: On Wed, Apr 1, 2015 at 1:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: It would absolutely *not* be reasonable for WHEN conditions for triggers on tables to work completely differently than they do for triggers on views. That ship's sailed. Clue me in, because I'm confused. If no trigger fires, we do whatever an object of that type would normally do in the absence of any trigger, no? For a view, that's error out; for a table, that's perform the action on the underlying data. That doesn't seem terribly unprincipled. I dunno about unprincipled; but we have already laid down the definition of INSTEAD OF triggers, and they act as I described. Read the code if you doubt it: which path is taken in ExecInsert depends only on whether INSTEAD OF triggers *exist* on the rel, not whether any of them actually fired (indeed, it would be difficult even to know that from here). I believe this was intentional, not just a coding artifact; it stems from having wanted to throw the error for uninsertable view well upstream of here, rather than having it be conditional on what happens at runtime. What I am objecting to is Andres' claim that it would be okay for INSTEAD OF triggers on tables to act completely differently in this regard from those on views. We have laid down the definition for views, and it is that nothing happens if the trigger exists but doesn't fire. 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] Why SyncRepWakeQueue is not static?
On Wed, Mar 25, 2015 at 10:07 PM, Tatsuo Ishii is...@postgresql.org wrote: Fix committed/pushed from master to 9.2. 9.1 declares it as a static function. Er, is that a good idea to back-patch that? Normally routine specs are maintained stable on back-branches, and this is just a cosmetic change. I'm not sure if it's a cosmetic change or not. I thought declaring to-be-static function as extern is against our coding standard. Moreover, if someone wants to change near the place in the source code in the future, changes made to head may not be easily back patched or cherry-picked to older branches if I do not back patch it. True. But if any third-party code calls that function, you just broke it. I don't think keeping the back-branches consistent with master is a sufficiently good reason for such a change. -- 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] possible dsm bug in dsm_attach()
On Wed, Mar 25, 2015 at 6:11 AM, Amit Kapila amit.kapil...@gmail.com wrote: I am facing an issue in case we need to create many segments for large inheritance hierarchy. Attached patch fixes the problem for me. Sigh. You'd think I'd be able to write a 30-line patch without introducing not one but two stupid bugs. But if you did think that, you'd be wrong. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] deparsing utility commands
On 2015-04-02 12:05:13 -0400, Robert Haas wrote: On Wed, Mar 25, 2015 at 3:21 PM, Ryan Pedela rped...@datalanche.com wrote: On Wed, Mar 25, 2015 at 11:59 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: * Should we prohibit DDL from within event triggers? Please don't prohibit DDL unless there is a really, really good reason to do so. I have several use cases in mind for event triggers, but they are only useful if I can perform DDL. For example, I want to create an Elasticsearch FDW and use that to index and search Postgres tables. When a table is created, I am planning to use an event trigger to capture the CREATE event and automatically create a foreign table via the Elasticsearch FDW. In addition, I would add normal triggers to the new table which capture DML and update the foreign table accordingly. In other words, I want to use FDWs and event triggers to automatically sync table DDL and DML to Elasticsearch. +1. I think that if it's unsafe to run DDL from with event triggers, then that might be a sign that it's not safe to run *any* user-defined code at that location. I think it's the job of the event trigger to make sure that it doesn't assume anything about what may have changed while the user-defined code was running. First of: I don't see a fundamental reason to forbid it, I think it's just simpler to analyze that way. But I'm unconvinced by that reasoning. As you know we prohibit executing DDL on some objects in *lots* of places c.f. CheckTableNotInUse(), without that imo being a sign of a more fundamental problem. Greetings, Andres Freund -- 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] POLA violation with \c service=
On 04/02/2015 12:42 PM, Alvaro Herrera wrote: David Fetter wrote: I haven't checked yet, but could this be because people aren't using --enable-depend with ./configure ? BTW --- No, this can't be the answer; --enable-depend is meant to help with recompiling after updating the source tree, but lack of it cannot cause any failures (assuming proper usage of make distclean). And the buildfarm always builds with pristine sources. cheers andrew -- 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] POLA violation with \c service=
David Fetter wrote: I haven't checked yet, but could this be because people aren't using --enable-depend with ./configure ? BTW --- No, this can't be the answer; --enable-depend is meant to help with recompiling after updating the source tree, but lack of it cannot cause any failures (assuming proper usage of make distclean). -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Something is rotten in the state of Denmark...
Robert Haas robertmh...@gmail.com writes: On Wed, Apr 1, 2015 at 7:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: I've not fully tracked it down, but I think that the blame falls on the MVCC-snapshots-for-catalog-scans patch; it appears that it's trying to read pg_am's pg_class entry with a snapshot that's too old, possibly because it assumes that sinval signaling is alive which I think ain't so. Hmm, sorry for the bug. It looks to me like sinval signaling gets started up at the beginning of InitPostgres(). Perhaps something like this? Yeah, it does, so you'd think this would work. I've now tracked down exactly what's happening, and it's this: while we're reading pg_authid during PerformAuthentication, we establish a catalog snapshot. Later on, we read pg_database to find out what DB we're connecting to. If any sinval messages for unshared system catalogs arrive at that point, they'll effectively be ignored, *because our MyDatabaseId is still zero and so doesn't match the incoming message*. That's okay as far as the catcaches are concerned, cause they're empty, and the relcache only knows about shared catalogs so far. But InvalidateCatalogSnapshot doesn't get called by LocalExecuteInvalidationMessage, and so we think we can plow ahead with the old snapshot. It looks to me like an appropriate fix would be as attached; thoughts? (I've tested this with a delay during relation lock acquisition and concurrent whole-database VACUUM FULLs, and so far been unable to break it.) regards, tom lane diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 1e646a1..6aa6868 100644 *** a/src/backend/utils/init/postinit.c --- b/src/backend/utils/init/postinit.c *** InitPostgres(const char *in_dbname, Oid *** 853,858 --- 853,866 MyProc-databaseId = MyDatabaseId; /* + * We may have already established a catalog snapshot while trying to read + * pg_authid; but until we have set up MyDatabaseId, we won't react to + * incoming sinval messages properly, so we won't realize it if the + * snapshot has been invalidated. Best to assume it's no good anymore. + */ + InvalidateCatalogSnapshot(); + + /* * Now, take a writer's lock on the database we are trying to connect to. * If there is a concurrently running DROP DATABASE on that database, this * will block us until it finishes (and has committed its update of -- 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] Abbreviated keys for Numeric
On Mon, Mar 23, 2015 at 9:46 PM, Peter Geoghegan p...@heroku.com wrote: On Mon, Mar 23, 2015 at 2:41 PM, Andrew Gierth and...@tao11.riddles.org.uk wrote: Peter As I said, I don't really consider that my patch is a rewrite, Peter especially V4, which changes nothing substantive except removing Peter 32-bit support. Well, that's a hell of an except. I guess you're right. I'm willing to go with whatever the consensus is on that question. So I changed my mind - I now think we should support 32-bit abbreviation. You might wonder why I've changed my mind so suddenly. It's not because I started caring about 32-bit systems overnight. For the record, I still think that they are almost irrelevant. But I've seen a new angle. One of the likely future uses for abbreviated keys is to guide B-Tree index scans. One technique that looks really interesting is storing an abbreviated key in internal pages. I always knew that abbreviation is as useful for index scans as it is for sorting - maybe more so. Reading Modern B-Tree techniques [1] again today, I realized that we can store fixed sized abbreviated keys in line items directly. If we stored a 4 byte abbreviated key, we'd pay no storage overhead on 64-bit systems that already lose that to ItemId alignment. We'd only generate abbreviated keys in internal B-Tree pages, during relatively infrequent page splits (internal pages naturally have a very wide spread of values anyway), so that would be a very low cost. Even when abbreviation doesn't work out, we have to visit the line item anyway, and an integer comparison on the same cacheline is effectively free. It would probably work out all the time anyway, owing to the natural spread of items within internal pages. Best of all, most index scans don't even need to look past the itemId array (for internal pages, which are the large majority visited by any given index scan, but a tiny minority of those actually stored), hugely cutting down on the number of cache misses. I could see this making index scans on numeric IndexTuples noticeably faster than even those on int8 IndexTuples. Of course, text is the type that this is really compelling for (perhaps int4 too - perhaps we could automatically get this for types that fit in 4 bytes naturally on 64-bit platforms). I'm not sure that we could do this with text without adopting ICU, which makes firm guarantees about binary sort key stability, so I thought that numeric could be considered a proof of concept for that. [1] http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.219.7269rep=rep1type=pdf -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] vac truncation scan problems
Kyotaro HORIGUCHI wrote: Hi, this is a bug in the commit 0d831389749a3baaced7b984205b9894a82444b9 . It allows vucuum freeze to be skipped and inversely lets regular vacuum wait for lock. The attched patch fixes it. In table_recheck_autovac, vacuum options are determined as following, tab-at_vacoptions = VACOPT_SKIPTOAST | (dovacuum ? VACOPT_VACUUM : 0) | (doanalyze ? VACOPT_ANALYZE : 0) | !(wraparound ? VACOPT_NOWAIT : 0); The line prefixed by '!' looks inverted. You're absolutely right. My mistake. Pushed your patch, thanks. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Additional role attributes superuser review
Robert, * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Apr 2, 2015 at 12:53 AM, Stephen Frost sfr...@snowman.net wrote: Clearly, further testing and documentation is required and I'll be getting to that over the next couple of days, but it's pretty darn late and I'm currently getting libpq undefined reference errors, probably because I need to set LD_LIBRARY_PATH, but I'm just too tired to now. :) Thoughts? The tricky part of this seems to me to be the pg_dump changes. The new catalog flag seems a little sketchy to me; wouldn't it be better to make the dump flag into an enum, DUMP_EVERYTHING, DUMP_ACL, DUMP_NONE? I agree that the pg_dump changes are a very big part of this change. I'll look at using an enum and see if that would work. Is this creating a user-visible API break, where pg_stat_activity and pg_stat_replication now always show only the publicly-visible information, and privileged users must explicitly decide to query the _all versions? If so, -1 from me on that part of this. Thanks for bringing it up. No, the existing API is exactly the same for the existing views- the only difference is that now there are _all versions which provide unfiltered data (but you have to have permission to call the _all() functions underneath, or you get a permission denied error). Where this does have an API change is with the simpler functions that used to do if (superuser() || replication_role()) and now depend on the GRANT system instead. We can't (today, at least) say: GRANT EXECUTE ON function_whatever() TO replication_roles; And have that be kept current as that role attribute is modified. I've not done it yet, but we could certainly have pg_dump dump out GRANTs for the *current* users which have that role attribute and give those users access to the functions via the normal permissions system. I'm not really sure that's a good idea though- it might be better to have a clean break here (and one which is clearly in the more secure/explicit direction) and tell admins in the release notes to GRANT EXECUTE on the functions for the roles they want to give permission to. We could also duplicate the functions and have the existing ones remain as-is and have the new ones just depend on the GRANT system, but I don't particularly like that since I'm afraid we'd end up in the same boat we're in now with pg_user and friends. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] vac truncation scan problems
Alvaro Herrera alvhe...@2ndquadrant.com writes: Kyotaro HORIGUCHI wrote: The line prefixed by '!' looks inverted. You're absolutely right. My mistake. Pushed your patch, thanks. Don't see any such commit from here? 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] Resetting crash time of background worker
On Sun, Mar 22, 2015 at 10:55 PM, Amit Khandekar amitdkhan...@gmail.com wrote: On 17 March 2015 at 19:12, Robert Haas robertmh...@gmail.com wrote: On Tue, Mar 17, 2015 at 1:33 AM, Amit Khandekar amitdkhan...@gmail.com wrote: I think we either have to retain the knowledge that the worker has crashed using some new field, or else, we should reset the crash time only if it is not flagged BGW_NEVER_RESTART. I think you're right, and I think we should do the second of those. Thanks for tracking this down. Thanks. Attached a patch accordingly. Put this into the June 2015 commitfest. Committed and back-patched to 9.4. -- 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] Something is rotten in the state of Denmark...
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 12:54 PM, Tom Lane t...@sss.pgh.pa.us wrote: It looks to me like an appropriate fix would be as attached; thoughts? Hmm, that fix doesn't reach as far as what I did. My proposal would regard a catalog snapshot as immediately stale, so if we're asked for a catalog snapshot multiple times before InitPostgres() is called, we'll take a new one every time. Your proposal invalidates them just once, after setting up MyDatabaseId. Assuming yours is safe, it's better, because it invalidates less aggressively. Right. The only thing I'm worried about is that I think PerformAuthentication() runs before InitPostgres(); sinval isn't working at all at that point, even for shared catalogs. No, PerformAuthentication is called by InitPostgres. However, I'm having second thoughts about whether we've fully diagnosed this. Three out of the four failures we've seen in the buildfarm reported cache lookup failed for access method 403, not could not open relation with OID 2601 ... and I'm so far only able to replicate the latter symptom. It's really unclear how the former one could arise, because nothing that vacuum.sql does would change xmin of the rows in pg_am. 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] Something is rotten in the state of Denmark...
On Thu, Apr 2, 2015 at 2:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 12:54 PM, Tom Lane t...@sss.pgh.pa.us wrote: It looks to me like an appropriate fix would be as attached; thoughts? Hmm, that fix doesn't reach as far as what I did. My proposal would regard a catalog snapshot as immediately stale, so if we're asked for a catalog snapshot multiple times before InitPostgres() is called, we'll take a new one every time. Your proposal invalidates them just once, after setting up MyDatabaseId. Assuming yours is safe, it's better, because it invalidates less aggressively. Right. The only thing I'm worried about is that I think PerformAuthentication() runs before InitPostgres(); sinval isn't working at all at that point, even for shared catalogs. No, PerformAuthentication is called by InitPostgres. Oops, OK. However, I'm having second thoughts about whether we've fully diagnosed this. Three out of the four failures we've seen in the buildfarm reported cache lookup failed for access method 403, not could not open relation with OID 2601 ... and I'm so far only able to replicate the latter symptom. It's really unclear how the former one could arise, because nothing that vacuum.sql does would change xmin of the rows in pg_am. It probably changes the *relfilenode* of pg_am, because it runs VACUUM FULL on that catalog. Perhaps some backend sees the old relfilenode value and tries to a heap scan, interpreting the now-truncated file as empty? -- 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] Re: [HACKERS] GSoC 2015 proposal: Improve the performance of “ALTER TABLE .. SET LOGGED / UNLOGGED” statement
On Wed, Mar 25, 2015 at 9:46 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: http://www.postgresql.org/message-id/ca+tgmozm+-0r7h0edpzzjbokvvq+gavkchmno4fypveccw-...@mail.gmail.com I like the idea of the feature a lot, but the proposal to which you refer here mentions some problems, which I'm curious how you think you might solve. (I don't have any good ideas myself, beyond what I mentioned there.) -- 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] Remove fsync ON/OFF as a visible option?
On Wed, Mar 25, 2015 at 3:45 PM, Jim Nasby jim.na...@bluetreble.com wrote: I see 3 settings that allow people to accidentally shoot themselves in the foot; fsync, wal_sync_method and full_page_writes. Those aren't even the top three in my experience, let alone the only three. -- 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] Zero-padding and zero-masking fixes for to_char(float)
On Wed, Apr 1, 2015 at 11:48:37AM -0400, Bruce Momjian wrote:
This should return something like 16.000... (per Oracle
output at the URL above, float4 has 6 significant digits on my compiler)
but I can't seem to figure how to get printf() to round non-fractional
parts. I am afraid the only solution is to use printf's %e format and
place the decimal point myself.
Hearing nothing, I went with the %e approach; patch attached. The new
output looks right:
test= SELECT to_char(float4
'15.9123456789123456789000',
repeat('9', 50) || '.' || repeat('9', 50));
to_char
16.00
(1 row)
The fact I still don't have a complete solution suggests this is 9.6
material but I still want to work on it so it is ready.
I will keep this patch for 9.6 unless I hear otherwise.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
new file mode 100644
index 40a353f..d283cd2
*** a/src/backend/utils/adt/formatting.c
--- b/src/backend/utils/adt/formatting.c
***
*** 113,125
#define DCH_MAX_ITEM_SIZ 12 /* max localized day name */
#define NUM_MAX_ITEM_SIZ 8 /* roman number (RN has 15 chars) */
- /* --
- * More is in float.c
- * --
- */
- #define MAXFLOATWIDTH 60
- #define MAXDOUBLEWIDTH 500
-
/* --
* Format parser structs
--- 113,118
*** static DCHCacheEntry *DCH_cache_getnew(c
*** 989,994
--- 982,989
static NUMCacheEntry *NUM_cache_search(char *str);
static NUMCacheEntry *NUM_cache_getnew(char *str);
static void NUM_cache_remove(NUMCacheEntry *ent);
+ static char *add_pre_zero_padding(char *num_str, int decimal_shift);
+ static char *add_post_zero_padding(char *num_str, int pad_digits);
/* --
*** do { \
*** 5016,5021
--- 5011,5103
SET_VARSIZE(result, len + VARHDRSZ); \
} while (0)
+ /*
+ * add_pre_zero_padding
+ *
+ * printf() can only round non-fractional parts of a number if the number
+ * is in exponential format, so this function takes a number in that format
+ * and adds pre-decimal zero padding.
+ */
+ static char *
+ add_pre_zero_padding(char *num_str, int decimal_shift)
+ {
+ /* one for decimal point, one for trailing null byte */
+ char *out = palloc(strlen(num_str) + decimal_shift + 1 + 1), *out_p = out;
+ char *num_str_p = num_str;
+ bool decimal_found = false;
+
+ /* copy the number before 'e' and shift the decimal point */
+ while (*num_str_p *num_str_p != 'e')
+ {
+ if (*num_str_p == '.')
+ {
+ num_str_p++;
+ decimal_found = true;
+ }
+ else
+ {
+ *(out_p++) = *(num_str_p++);
+ if (decimal_found)
+ decimal_shift--;
+ }
+ }
+
+ /* add zero pad digits */
+ while (decimal_shift-- 0)
+ *(out_p++) = '0';
+
+ *(out_p++) = '\0';
+
+ pfree(num_str);
+ return out;
+ }
+
+ /*
+ * add_post_zero_padding
+ *
+ * Some sprintf() implementations have a 512-digit precision limit, and we
+ * need sprintf() to round to the internal precision, so this function adds
+ * zero padding between the mantissa and exponent of an exponential-format
+ * number, or after the supplied string for non-exponent strings.
+ */
+ static char *
+ add_post_zero_padding(char *num_str, int pad_digits)
+ {
+ /* one for decimal point, one for trailing null byte */
+ char *out = palloc(strlen(num_str) + pad_digits + 1 + 1), *out_p = out;
+ char *num_str_p = num_str;
+ bool decimal_found = false;
+
+ /* copy the number before 'e', or the entire string if no 'e' */
+ while (*num_str_p *num_str_p != 'e')
+ {
+ if (*num_str_p == '.')
+ decimal_found = true;
+ *(out_p++) = *(num_str_p++);
+ }
+
+ /* add zero pad digits */
+ while (pad_digits-- 0)
+ {
+ if (!decimal_found)
+ {
+ *(out_p++) = '.';
+ decimal_found = true;
+ }
+
+ *(out_p++) = '0';
+ }
+
+ /* copy 'e' and everything after */
+ while (*num_str_p)
+ *(out_p++) = *(num_str_p++);
+
+ *(out_p++) = '\0';
+
+ pfree(num_str);
+ return out;
+ }
+
/* ---
* NUMERIC to_number() (convert string to numeric)
* ---
*** int4_to_char(PG_FUNCTION_ARGS)
*** 5214,5221
/* we can do it easily because float8 won't lose any precision */
float8 val = (float8) value;
! orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
! snprintf(orgnum, MAXDOUBLEWIDTH + 1, %+.*e, Num.post, val);
/*
* Swap a leading positive sign for a space.
--- 5296,5307
Re: [HACKERS] Abbreviated keys for Numeric
On Tue, Mar 24, 2015 at 3:03 AM, Andrew Gierth
and...@tao11.riddles.org.uk wrote:
So here's the latest (and, hopefully, last) version:
- adds diagnostic output from numeric_abbrev_abort using the trace_sort
GUC
- fixed Datum cs. uint32 issues in hash_uint32
- added a short comment about excess-k representation
- tweaked the indenting and comments a bit
I'm not particularly committed to any specific way of handling the
DEC_DIGITS issue. (I moved away from the transparently skip
abbreviations approach of the original because it seemed that reducing
#ifdefism in the code was a desirable feature.)
The INT64_MIN/MAX changes should be committed fairly soon. (I haven't
posted a patch for TRACE_SORT)
I think this is really nice work, so I have committed this version. I
made a few fairly minor changes, hopefully without breaking anything
in the process:
- I adjusted things for recent commits around INT{32,63}_{MIN_MAX}.
- I removed (void) ssup; which I do not think is normal PostgreSQL style.
- Removed the #if DEC_DIGITS != 4 check. The comment is great, but I
think we don't need protect against #if 0 code get re-enabled.
- I removed the too-clever (at least IMHO) handing of TRACE_SORT in
favor of just using #ifdef around each occurrence.
- I also declared trace_sort in guc.h, where various other GUCs are
declared, instead of declaring it privately in each file that needed
it.
- Changed some definitions to depend on SIZEOF_DATUM rather than
USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please check it.
- Fixed an OID conflict.
- And of course, bumped catversion.
Thanks,
--
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] Something is rotten in the state of Denmark...
On Thu, Apr 2, 2015 at 12:54 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Apr 1, 2015 at 7:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: I've not fully tracked it down, but I think that the blame falls on the MVCC-snapshots-for-catalog-scans patch; it appears that it's trying to read pg_am's pg_class entry with a snapshot that's too old, possibly because it assumes that sinval signaling is alive which I think ain't so. Hmm, sorry for the bug. It looks to me like sinval signaling gets started up at the beginning of InitPostgres(). Perhaps something like this? Yeah, it does, so you'd think this would work. I've now tracked down exactly what's happening, and it's this: while we're reading pg_authid during PerformAuthentication, we establish a catalog snapshot. Later on, we read pg_database to find out what DB we're connecting to. If any sinval messages for unshared system catalogs arrive at that point, they'll effectively be ignored, *because our MyDatabaseId is still zero and so doesn't match the incoming message*. That's okay as far as the catcaches are concerned, cause they're empty, and the relcache only knows about shared catalogs so far. But InvalidateCatalogSnapshot doesn't get called by LocalExecuteInvalidationMessage, and so we think we can plow ahead with the old snapshot. It looks to me like an appropriate fix would be as attached; thoughts? (I've tested this with a delay during relation lock acquisition and concurrent whole-database VACUUM FULLs, and so far been unable to break it.) Hmm, that fix doesn't reach as far as what I did. My proposal would regard a catalog snapshot as immediately stale, so if we're asked for a catalog snapshot multiple times before InitPostgres() is called, we'll take a new one every time. Your proposal invalidates them just once, after setting up MyDatabaseId. Assuming yours is safe, it's better, because it invalidates less aggressively. The only thing I'm worried about is that I think PerformAuthentication() runs before InitPostgres(); sinval isn't working at all at that point, even for shared catalogs. If we were to lock a relation, build a relcache entry, unlock it, and then do that again, all before SharedInvalBackendInit(false) is called, what would keep us from failing to notice an intervening invalidation? Maybe there is something, because otherwise this was probably broken even before the MVCC-catalog-snapshots patch, but I don't know what it is. -- 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] Fix pgbench --progress report under (very) low rate
On Sun, Mar 22, 2015 at 6:12 AM, Fabien COELHO coe...@cri.ensmp.fr wrote: When running with low rate, the --progress is only printed when there is some activity, which makes it quite irregular, including some catching up with stupid tps figures. Shame on me for this feature (aka bug) in the first place. This patch fixes this behavior by considering the next report time as a target to meet as well as transaction schedule times. Before the patch: sh ./pgbench -R 0.5 -T 10 -P 1 -S progress: 1.7 s, 0.6 tps, lat 6.028 ms stddev -nan, lag 1.883 ms progress: 2.2 s, 2.3 tps, lat 2.059 ms stddev -nan, lag 0.530 ms progress: 7.3 s, 0.4 tps, lat 2.944 ms stddev 1.192, lag 2.624 ms progress: 7.3 s, 1402.5 tps, lat 5.115 ms stddev 0.000, lag 0.000 ms progress: 7.3 s, 0.0 tps, lat -nan ms stddev -nan, lag inf ms progress: 7.3 s, 335.2 tps, lat 3.106 ms stddev 0.000, lag 0.000 ms progress: 8.8 s, 0.0 tps, lat -nan ms stddev -nan, lag inf ms progress: 8.8 s, 307.6 tps, lat 4.855 ms stddev -nan, lag 0.000 ms progress: 10.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms After the patch: sh ./pgbench -R 0.5 -T 10 -P 1 -S progress: 1.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms progress: 2.0 s, 1.0 tps, lat 5.980 ms stddev 0.000, lag 0.733 ms progress: 3.0 s, 1.0 tps, lat 1.905 ms stddev 0.000, lag 0.935 ms progress: 4.0 s, 1.0 tps, lat 3.966 ms stddev 0.000, lag 0.623 ms progress: 5.0 s, 2.0 tps, lat 2.527 ms stddev 1.579, lag 0.512 ms progress: 6.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms progress: 7.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms progress: 8.0 s, 1.0 tps, lat 1.750 ms stddev 0.000, lag 0.767 ms progress: 9.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms progress: 10.0 s, 2.0 tps, lat 2.403 ms stddev 1.386, lag 0.357 ms I haven't had time to really review the code here (except to notice that you have a typo: nedded) but the idea of it seems good. -- 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] Something is rotten in the state of Denmark...
On Thu, Apr 2, 2015 at 3:59 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 2:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Actually, now that I think it through, the could not open relation error is pretty odd in itself. If we are trying to open pg_am using a stale catalog snapshot, it seems like we ought to reliably find its old pg_class tuple (the one with the obsolete relfilenode), rather than finding nothing. But the latter is the behavior I'm seeing. What's to stop the old tuple from being HOT-pruned? Hm, that may be it. I went back to the previous test scenario, and now I can *only* get the cache lookup failed for access method behavior, instead of what I was getting before, so I'm getting a bit confused :-(. However, it does seem clear that the mechanism is indeed that we're relying on an obsolete copy of pg_am's pg_class tuple, hence scanning a truncated relfilenode, and that the patch I proposed fixes it. Perhaps the difference has to do with whether pg_am's pg_class tuple is on a page that hasn't got enough room for a HOT update? But I definitely tried it several times and consistently got the same failure before. That seems plausible, except that I have no idea why that would vary from one test setup to another. I suggest pushing the patch you proposed upthread and seeing what the buildfarm thinks. -- 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
