Re: [HACKERS] WIP: multivariate statistics / proof of concept
Tomas Vondra wrote: attached is a WIP patch implementing multivariate statistics. I think that is pretty useful. Oracle has an identical feature called extended statistics. That's probably an entirely different thing, but it would be very nice to have statistics to estimate the correlation between columns of different tables, to improve the estimate for the number of rows in a join. Yours, Laurenz Albe -- 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] [9.4 bug] The database server hangs with write-heavy workload on Windows
On 10/10/2014 05:08 PM, MauMau wrote: From: Craig Ringer cr...@2ndquadrant.com It sounds like they've produced a test case, so they should be able to with a bit of luck. Or even better, send you the test case. I asked the user about this. It sounds like the relevant test case consists of many scripts. He explained to me that the simplified test steps are: 1. initdb 2. pg_ctl start 3. Create 16 tables. Each of those tables consist of around 10 columns. 4. Insert 1000 rows into each of those 16 tables. 5. Launch 16 psql sessions concurrently. Each session updates all 1000 rows of one table, e.g., session 1 updates table 1, session 2 updates table 2, and so on. 6. Repeat step 5 50 times. This sounds a bit complicated, but I understood that the core part is 16 concurrent updates, which should lead to contention on xlog insert slots and/or spinlocks. I was able to reproduce this. I reduced wal_buffers to 64kB, and NUM_XLOGINSERT_LOCKS to 4 to increase the probability of the deadlock, and ran a test case as above on my laptop for several hours, and it finally hung. Will investigate... - 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] Proposal : REINDEX SCHEMA
On Mon, Oct 13, 2014 at 1:25 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Sun, Oct 12, 2014 at 2:27 PM, Stephen Frost sfr...@snowman.net wrote: * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Sawada Masahiko wrote: Attached WIP patch adds new syntax REINEX SCHEMA which does reindexing all table of specified schema. There are syntax dose reindexing specified index, per table and per database, but we can not do reindexing per schema for now. It seems doubtful that there really is much use for this feature, but if there is, I think a better syntax precedent is the new ALTER TABLE ALL IN TABLESPACE thingy, rather than your proposed REINDEX SCHEMA. Something like REINDEX TABLE ALL IN SCHEMA perhaps. Yeah, I tend to agree that we should be looking at the 'ALL IN TABLESPACE' and 'ALL IN SCHEMA' type of commands to keep things consistent. This might be an alternative for the vacuum / analyze / reindex database commands also.. Some review: 1) +1 to REINDEX ALL IN SCHEMA name Thank you for comment and reviewing! I agree with this. I will change syntax to above like that. 2) IMHO the logic should be exactly the same as REINDEX DATABASE, including the transaction control. Imagine a schema with a lot of tables, you can lead to a deadlock using just one transaction block. Yep, it should be same as REINDEX DATABASE. IMO, we can put them into one function if they use exactly same logic. Thought? 3) The patch was applied to master and compile without warnings 4) Typo (... does not have any table) + if (!reindex_schema(heapOid)) + ereport(NOTICE, + (errmsg(schema\%s\ does not hava any table, + schema-relname))); Thanks! I will correct typo. 5) Missing of regression tests, please add it to src/test/regress/sql/create_index.sql 6) You need to add psql complete tabs Next version patch, that I will submit, will have 5), 6) things you pointed. 7) I think we can add -S / --schema option do reindexdb in this patch too. What do you think? +1 to add -S / --schema option I was misunderstanding about that. I will make the patch which adds this option as separate patch. -- Regards, --- Sawada Masahiko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tracking commit timestamps
On 09/09/14 19:05, Petr Jelinek wrote: Hi, I worked bit on this patch to make it closer to committable state. There are several bugs fixed, including ones mentioned by Jamie (writing WAL during recovery). Also support for pg_resetxlog/pg_upgrade has been implemented by Andres. I added simple regression test and regression contrib module to cover both off and on settings. The SLRU issue Heikki mentioned should be also gone mainly thanks to 638cf09e7 (I did test it too). Here is updated version that works with current HEAD for the October committfest. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c index 3b8241b..f0a023f 100644 --- a/contrib/pg_upgrade/pg_upgrade.c +++ b/contrib/pg_upgrade/pg_upgrade.c @@ -423,8 +423,10 @@ copy_clog_xlog_xid(void) /* set the next transaction id and epoch of the new cluster */ prep_status(Setting next transaction ID and epoch for new cluster); exec_prog(UTILITY_LOG_FILE, NULL, true, - \%s/pg_resetxlog\ -f -x %u \%s\, - new_cluster.bindir, old_cluster.controldata.chkpnt_nxtxid, + \%s/pg_resetxlog\ -f -x %u -c %u \%s\, + new_cluster.bindir, + old_cluster.controldata.chkpnt_nxtxid, + old_cluster.controldata.chkpnt_nxtxid, new_cluster.pgdata); exec_prog(UTILITY_LOG_FILE, NULL, true, \%s/pg_resetxlog\ -f -e %u \%s\, diff --git a/contrib/pg_xlogdump/rmgrdesc.c b/contrib/pg_xlogdump/rmgrdesc.c index bfb3573..c0a0409 100644 --- a/contrib/pg_xlogdump/rmgrdesc.c +++ b/contrib/pg_xlogdump/rmgrdesc.c @@ -9,6 +9,7 @@ #include postgres.h #include access/clog.h +#include access/committs.h #include access/gin.h #include access/gist_private.h #include access/hash.h diff --git a/contrib/test_committs/.gitignore b/contrib/test_committs/.gitignore new file mode 100644 index 000..1f95503 --- /dev/null +++ b/contrib/test_committs/.gitignore @@ -0,0 +1,5 @@ +# Generated subdirectories +/log/ +/isolation_output/ +/regression_output/ +/tmp_check/ diff --git a/contrib/test_committs/Makefile b/contrib/test_committs/Makefile new file mode 100644 index 000..2240749 --- /dev/null +++ b/contrib/test_committs/Makefile @@ -0,0 +1,45 @@ +# Note: because we don't tell the Makefile there are any regression tests, +# we have to clean those result files explicitly +EXTRA_CLEAN = $(pg_regress_clean_files) ./regression_output ./isolation_output + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/test_committs +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif + +# We can't support installcheck because normally installcheck users don't have +# the required track_commit_timestamp on +installcheck:; + +check: regresscheck + +submake-regress: + $(MAKE) -C $(top_builddir)/src/test/regress all + +submake-test_committs: + $(MAKE) -C $(top_builddir)/contrib/test_committs + +REGRESSCHECKS=committs_on + +regresscheck: all | submake-regress submake-test_committs + $(MKDIR_P) regression_output + $(pg_regress_check) \ + --temp-config $(top_srcdir)/contrib/test_committs/committs.conf \ + --temp-install=./tmp_check \ + --extra-install=contrib/test_committs \ + --outputdir=./regression_output \ + $(REGRESSCHECKS) + +regresscheck-install-force: | submake-regress submake-test_committs + $(pg_regress_installcheck) \ + --extra-install=contrib/test_committs \ + $(REGRESSCHECKS) + +PHONY: submake-test_committs submake-regress check \ + regresscheck regresscheck-install-force \ No newline at end of file diff --git a/contrib/test_committs/committs.conf b/contrib/test_committs/committs.conf new file mode 100644 index 000..d221a60 --- /dev/null +++ b/contrib/test_committs/committs.conf @@ -0,0 +1 @@ +track_commit_timestamp = on \ No newline at end of file diff --git a/contrib/test_committs/expected/committs_on.out b/contrib/test_committs/expected/committs_on.out new file mode 100644 index 000..9920343 --- /dev/null +++ b/contrib/test_committs/expected/committs_on.out @@ -0,0 +1,21 @@ +-- +-- Commit Timestamp (on) +-- +CREATE TABLE committs_test(id serial, ts timestamptz default now()); +INSERT INTO committs_test DEFAULT VALUES; +INSERT INTO committs_test DEFAULT VALUES; +INSERT INTO committs_test DEFAULT VALUES; +SELECT id, pg_get_transaction_extradata(xmin), + pg_get_transaction_committime(xmin) = ts, + pg_get_transaction_committime(xmin) now(), + pg_get_transaction_committime(xmin) - ts '60s' -- 60s should give a lot of reserve +FROM committs_test +ORDER BY id; + id | pg_get_transaction_extradata | ?column? | ?column? | ?column? ++--+--+--+-- + 1 |0 | t| t| t + 2 |0 | t| t| t + 3
Re: [HACKERS] Is analyze_new_cluster.sh still useful?
Re: Bruce Momjian 2014-10-12 20141011224002.gm21...@momjian.us I have applied a patch to 9.5 to output ./ as a prefix for Unix script file names. While this also works on Windows, it is likely to be confusing. The new Unix output is: Upgrade Complete Optimizer statistics are not transferred by pg_upgrade so, once you start the new server, consider running: ./analyze_new_cluster.sh Running this script will delete the old cluster's data files: ./delete_old_cluster.sh I like this, thanks! Mit freundlichen Grüßen, Christoph Berg -- Senior Berater, Tel.: +49 (0)21 61 / 46 43-187 credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209 Hohenzollernstr. 133, 41061 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer pgp fingerprint: 5C48 FE61 57F4 9179 5970 87C6 4C5A 6BAB 12D2 A7AE -- 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] Sequence Access Method WIP
Hi,
I rewrote the patch with different API along the lines of what was
discussed.
The API now consists of following functions:
sequence_alloc - allocating range of new values
The function receives the sequence relation, current value, number of
requested values amdata and relevant sequence options like min/max and
returns new amdata, new current value, number of values allocated and
also if it needs wal write (that should be returned if amdata has
changed plus other reasons the AM might have to force the wal update).
sequence_setval - notification that setval is happening
This function gets sequence relation, previous value and new value plus
the amdata and returns amdata (I can imagine some complex sequence AMs
will want to throw error that setval can't be done on them).
sequence_request_update/sequence_update - used for background processing
Basically AM can call the sequence_request_update and backend will then
call the sequence_update method of an AM with current amdata and will
write the updated amdata to disk
sequence_seqparams - function to process/validate the standard sequence
options like start position, min/max, increment by etc by the AM, it's
called in addition to the standard processing
sequence_reloptions - this is the only thing that remained unchanged
from previous patch, it's meant to pass custom options to the AM
Only the alloc and reloptions methods are required (and implemented by
the local AM).
The caching, xlog writing, updating the page, etc is handled by backend,
the AM does not see the tuple at all. I decided to not pass even the
struct around and just pass the relevant options because I think if we
want to abstract the storage properly then the AM should not care about
how the pg_sequence looks like at all, even if it means that the
sequence_alloc parameter list is bit long.
For the amdata handling (which is the AM's private data variable) the
API assumes that (Datum) 0 is NULL, this seems to work well for
reloptions so should work here also and it simplifies things a little
compared to passing pointers to pointers around and making sure
everything is allocated, etc.
Sadly the fact that amdata is not fixed size and can be NULL made the
page updates of the sequence relation quite more complex that it used to
be. There are probably some optimizations possible there but I think the
patch is good enough for the review now, so I am adding it to October
commitfest.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/src/backend/access/Makefile b/src/backend/access/Makefile
index c32088f..aea4a14 100644
--- a/src/backend/access/Makefile
+++ b/src/backend/access/Makefile
@@ -8,6 +8,6 @@ subdir = src/backend/access
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = common gin gist hash heap index nbtree rmgrdesc spgist transam
+SUBDIRS = common gin gist hash heap index nbtree rmgrdesc spgist transam sequence
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index e0b81b9..c5b7e0a 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -306,6 +306,7 @@ static bool need_initialization = true;
static void initialize_reloptions(void);
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+static bytea *common_am_reloptions(RegProcedure amoptions, Datum reloptions, bool validate);
/*
* initialize_reloptions
@@ -806,7 +807,8 @@ untransformRelOptions(Datum options)
* instead.
*
* tupdesc is pg_class' tuple descriptor. amoptions is the amoptions regproc
- * in the case of the tuple corresponding to an index, or InvalidOid otherwise.
+ * in the case of the tuple corresponding to an index or sequence, InvalidOid
+ * otherwise.
*/
bytea *
extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, Oid amoptions)
@@ -839,6 +841,9 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, Oid amoptions)
case RELKIND_INDEX:
options = index_reloptions(amoptions, datum, false);
break;
+ case RELKIND_SEQUENCE:
+ options = sequence_reloptions(amoptions, datum, false);
+ break;
case RELKIND_FOREIGN_TABLE:
options = NULL;
break;
@@ -1284,13 +1289,31 @@ heap_reloptions(char relkind, Datum reloptions, bool validate)
/*
* Parse options for indexes.
+ */
+bytea *
+index_reloptions(RegProcedure amoptions, Datum reloptions, bool validate)
+{
+ return common_am_reloptions(amoptions, reloptions, validate);
+}
+
+/*
+ * Parse options for sequences.
+ */
+bytea *
+sequence_reloptions(RegProcedure amoptions, Datum reloptions, bool validate)
+{
+ return common_am_reloptions(amoptions, reloptions, validate);
+}
+
+/*
+ * Parse options for indexes or sequences.
*
* amoptions Oid of option parser
* reloptions options as text[]
Re: [HACKERS] JsonbValue to Jsonb conversion
Hi I am working on review of this patch. There is new warnings: jsonb.c: In function ‘jsonb_agg_transfn’: jsonb.c:1540:20: warning: assignment makes pointer from integer without a cast v.val.numeric = DirectFunctionCall1(numeric_uplus, NumericGetDatum(v.val.numeric)); ^ jsonb.c: In function ‘jsonb_object_agg_transfn’: jsonb.c:1745:20: warning: assignment makes pointer from integer without a cast v.val.numeric = DirectFunctionCall1(numeric_uplus, NumericGetDatum(v.val.numeric)); [pavel@localhost postgresql]$ gcc --version gcc (GCC) 4.9.1 20140930 (Red Hat 4.9.1-11) Check fails parallel group (19 tests): alter_table plancache temp domain prepare limit plpgsql conversion sequence copy2 rangefuncs returning truncate xml with without_oid largeobject polymorphism rowtypes plancache... FAILED (test process exited with exit code 2) limit... FAILED (test process exited with exit code 2) plpgsql ... FAILED (test process exited with exit code 2) copy2... FAILED (test process exited with exit code 2) temp ... FAILED (test process exited with exit code 2) domain ... FAILED (test process exited with exit code 2) rangefuncs ... FAILED (test process exited with exit code 2) prepare ... FAILED (test process exited with exit code 2) without_oid ... FAILED (test process exited with exit code 2) conversion ... FAILED (test process exited with exit code 2) truncate ... FAILED (test process exited with exit code 2) alter_table ... FAILED (test process exited with exit code 2) sequence ... FAILED (test process exited with exit code 2) polymorphism ... FAILED (test process exited with exit code 2) rowtypes ... FAILED (test process exited with exit code 2) returning... FAILED (test process exited with exit code 2) largeobject ... FAILED (test process exited with exit code 2) with ... FAILED (test process exited with exit code 2) xml ... FAILED (test process exited with exit code 2) test stats... FAILED (test process exited with exit code 2) [pavel@localhost postgresql]$ uname -a Linux localhost.localdomain 3.16.3-302.fc21.x86_64 #1 SMP Fri Sep 26 14:27:20 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux backtrace Core was generated by `postgres: pavel regression [local] SELECT '. Program terminated with signal SIGSEGV, Segmentation fault. (gdb) bt #0 0x01e95300 in ?? () #1 0x007c048b in parse_object_field (lex=0x1ede9d8, sem=0x7fff3c3c4660) at json.c:398 #2 0x007c0524 in parse_object (lex=0x1ede9d8, sem=0x7fff3c3c4660) at json.c:430 #3 0x007c0214 in pg_parse_json (lex=0x1ede9d8, sem=0x7fff3c3c4660) at json.c:297 #4 0x007c5d91 in datum_to_jsonb (val=32118224, is_null=0 '\000', result=0x7fff3c3c4800, tcategory=JSONBTYPE_JSON, outfuncoid=322, key_scalar=0 '\000') at jsonb.c:789 #5 0x007c68be in add_jsonb (val=32118224, is_null=0 '\000', result=0x7fff3c3c4800, val_type=114, key_scalar=0 '\000') at jsonb.c:1050 #6 0x007c6d08 in jsonb_build_object (fcinfo=0x1edcb80) at jsonb.c:1155 #7 0x0060bfc5 in ExecMakeFunctionResultNoSets (fcache=0x1edcb10, econtext=0x1edc920, isNull=0x1edd568 , isDone=0x1edd680) at execQual.c:1992 #8 0x0060c8bc in ExecEvalFunc (fcache=0x1edcb10, econtext=0x1edc920, isNull=0x1edd568 , isDone=0x1edd680) at execQual.c:2383 #9 0x00612869 in ExecTargetList (targetlist=0x1edd650, econtext=0x1edc920, values=0x1edd550, isnull=0x1edd568 , itemIsDone=0x1edd680, isDone=0x7fff3c3c4a84) at execQual.c:5265 #10 0x00612e9d in ExecProject (projInfo=0x1edd580, isDone=0x7fff3c3c4a84) at execQual.c:5480 #11 0x0062c046 in ExecResult (node=0x1edc810) at nodeResult.c:155 #12 0x00608997 in ExecProcNode (node=0x1edc810) at execProcnode.c:373 #13 0x0060696e in ExecutePlan (estate=0x1edc700, planstate=0x1edc810, operation=CMD_SELECT, sendTuples=1 '\001', numberTuples=0, direction=ForwardScanDirection, dest=0x1ea18b0) at execMain.c:1481 #14 0x00604de8 in standard_ExecutorRun (queryDesc=0x1edc2f0, direction=ForwardScanDirection, count=0) at execMain.c:308 #15 0x00604ce5 in ExecutorRun (queryDesc=0x1edc2f0, direction=ForwardScanDirection, count=0) at execMain.c:256 #16 0x0075615a in PortalRunSelect (portal=0x1eda2e0, forward=1 '\001', count=0, dest=0x1ea18b0) at pquery.c:946 #17 0x00755e34 in PortalRun (portal=0x1eda2e0, count=9223372036854775807, isTopLevel=1 '\001', dest=0x1ea18b0, altdest=0x1ea18b0, completionTag=0x7fff3c3c4dc0 ) at pquery.c:790 #18 0x007502c2 in
Re: [HACKERS] split builtins.h to quote.h
On Sat, Oct 11, 2014 at 6:09 PM, Stephen Frost sfr...@snowman.net wrote: * Noah Misch (n...@leadboat.com) wrote: On Sat, Oct 11, 2014 at 11:43:46PM +0200, Andres Freund wrote: I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Agreed. If the patch preserved compatibility by having builtins.h include quote.h, I would not object. That seems reasonable to me also- though I'd caveat it as for now and make sure to make a note of the reason it's included in the comments... Yuck. I think if we're going to break it, we should just break it. No significant advantage will be gained by splitting it out and then #including it; nobody's really going to fix their module builds until they actually break. On the general substance of the issue, our usual convention is that src/backend/X/Y/Z.c has its prototypes in src/include/X/Z.h. If this proposal moves us closer to that, I'm OK with enduring the module breakage that will result. If, on the other hand, it in moves us further away from that, then I'm against it. What I find strange about the actual patch is that it moves some but not all of the prototypes for the stuff that ends up in quote.c into quote.h. That doesn't seem right. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal : REINDEX SCHEMA
On Mon, Oct 13, 2014 at 5:39 AM, Sawada Masahiko sawada.m...@gmail.com wrote: On Mon, Oct 13, 2014 at 1:25 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Sun, Oct 12, 2014 at 2:27 PM, Stephen Frost sfr...@snowman.net wrote: * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Sawada Masahiko wrote: Attached WIP patch adds new syntax REINEX SCHEMA which does reindexing all table of specified schema. There are syntax dose reindexing specified index, per table and per database, but we can not do reindexing per schema for now. It seems doubtful that there really is much use for this feature, but if there is, I think a better syntax precedent is the new ALTER TABLE ALL IN TABLESPACE thingy, rather than your proposed REINDEX SCHEMA. Something like REINDEX TABLE ALL IN SCHEMA perhaps. Yeah, I tend to agree that we should be looking at the 'ALL IN TABLESPACE' and 'ALL IN SCHEMA' type of commands to keep things consistent. This might be an alternative for the vacuum / analyze / reindex database commands also.. Some review: 1) +1 to REINDEX ALL IN SCHEMA name Thank you for comment and reviewing! I agree with this. I will change syntax to above like that. 2) IMHO the logic should be exactly the same as REINDEX DATABASE, including the transaction control. Imagine a schema with a lot of tables, you can lead to a deadlock using just one transaction block. Yep, it should be same as REINDEX DATABASE. IMO, we can put them into one function if they use exactly same logic. Thought? 3) The patch was applied to master and compile without warnings 4) Typo (... does not have any table) + if (!reindex_schema(heapOid)) + ereport(NOTICE, + (errmsg(schema\%s\ does not hava any table, + schema-relname))); Thanks! I will correct typo. 5) Missing of regression tests, please add it to src/test/regress/sql/create_index.sql 6) You need to add psql complete tabs Next version patch, that I will submit, will have 5), 6) things you pointed. 7) I think we can add -S / --schema option do reindexdb in this patch too. What do you think? +1 to add -S / --schema option I was misunderstanding about that. I will make the patch which adds this option as separate patch. I registered to the next commitfest [1] and put myself as reviewer. Regards, [1] https://commitfest.postgresql.org/action/patch_view?id=1598 -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello
Re: [HACKERS] split builtins.h to quote.h
On Mon, Oct 13, 2014 at 10:04 PM, Robert Haas robertmh...@gmail.com wrote: On Sat, Oct 11, 2014 at 6:09 PM, Stephen Frost sfr...@snowman.net wrote: * Noah Misch (n...@leadboat.com) wrote: On Sat, Oct 11, 2014 at 11:43:46PM +0200, Andres Freund wrote: I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Agreed. If the patch preserved compatibility by having builtins.h include quote.h, I would not object. That seems reasonable to me also- though I'd caveat it as for now and make sure to make a note of the reason it's included in the comments... Yuck. I think if we're going to break it, we should just break it. No significant advantage will be gained by splitting it out and then #including it; nobody's really going to fix their module builds until they actually break. On the general substance of the issue, our usual convention is that src/backend/X/Y/Z.c has its prototypes in src/include/X/Z.h. If this proposal moves us closer to that, I'm OK with enduring the module breakage that will result. If, on the other hand, it in moves us further away from that, then I'm against it. That's a 2/2 tie then AFAIK: Noah and Stephen express concerns about the breakage, you and I would be fine with a clear breakage to make code more organized (correct me if you don't feel this way). What I find strange about the actual patch is that it moves some but not all of the prototypes for the stuff that ends up in quote.c into quote.h. That doesn't seem right. Are you referring to the Datum quote_*(PG_FUNCTION_ARGS) that are still let in builtins.h? That was let on purpose to let all the SQL functions within builtins.h but I'd be happy to move everything to quote.h to make the separation clearer. -- 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] split builtins.h to quote.h
* Robert Haas (robertmh...@gmail.com) wrote: On Sat, Oct 11, 2014 at 6:09 PM, Stephen Frost sfr...@snowman.net wrote: * Noah Misch (n...@leadboat.com) wrote: On Sat, Oct 11, 2014 at 11:43:46PM +0200, Andres Freund wrote: I personally wouldn't object plaing a #include for the splitof file into builtin.h to address backward compat concerns. Would imo still be an improvement. Agreed. If the patch preserved compatibility by having builtins.h include quote.h, I would not object. That seems reasonable to me also- though I'd caveat it as for now and make sure to make a note of the reason it's included in the comments... Yuck. I think if we're going to break it, we should just break it. I'm fine with that, for my part- was simply looking for a compromise, and having a deprecated period of time seemed reasonable. No significant advantage will be gained by splitting it out and then #including it; nobody's really going to fix their module builds until they actually break. Well, hopefully folks on -hackers would, though I agree that others aren't likely to. What I find strange about the actual patch is that it moves some but not all of the prototypes for the stuff that ends up in quote.c into quote.h. That doesn't seem right. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] jsonb generator functions
On 09/26/2014 04:54 PM, Andrew Dunstan wrote:
Here is a patch for the generator and aggregate functions for jsonb
that we didn't manage to get done in time for 9.4. They are all
equivalents of the similarly names json functions. Included are
to_jsonb
jsonb_build_object
jsonb_build_array
jsonb_object
jsonb_agg
jsonb_object_agg
Still to come: documentation.
Adding to the next commitfest.
Revised patch to fix compiler warnings.
cheers
andrew
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 2fd87fc..2712761 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -12,11 +12,20 @@
*/
#include postgres.h
+#include miscadmin.h
+#include access/htup_details.h
+#include access/transam.h
+#include catalog/pg_cast.h
+#include catalog/pg_type.h
#include libpq/pqformat.h
#include utils/builtins.h
+#include utils/datetime.h
+#include utils/lsyscache.h
#include utils/json.h
#include utils/jsonapi.h
#include utils/jsonb.h
+#include utils/syscache.h
+#include utils/typcache.h
typedef struct JsonbInState
{
@@ -24,6 +33,23 @@ typedef struct JsonbInState
JsonbValue *res;
} JsonbInState;
+/* unlike with json categories, we need to treat json and jsonb differently */
+typedef enum /* type categories for datum_to_jsonb */
+{
+ JSONBTYPE_NULL,/* null, so we didn't bother to identify */
+ JSONBTYPE_BOOL,/* boolean (built-in types only) */
+ JSONBTYPE_NUMERIC, /* numeric (ditto) */
+ JSONBTYPE_TIMESTAMP, /* we use special formatting for timestamp */
+ JSONBTYPE_TIMESTAMPTZ, /* ... and timestamptz */
+ JSONBTYPE_JSON,/* JSON */
+ JSONBTYPE_JSONB, /* JSONB */
+ JSONBTYPE_ARRAY, /* array */
+ JSONBTYPE_COMPOSITE, /* composite */
+ JSONBTYPE_JSONCAST, /* something with an explicit cast to JSON */
+ JSONBTYPE_JSONBCAST, /* something with an explicit cast to JSONB */
+ JSONBTYPE_OTHER/* all else */
+} JsonbTypeCategory;
+
static inline Datum jsonb_from_cstring(char *json, int len);
static size_t checkStringLen(size_t len);
static void jsonb_in_object_start(void *pstate);
@@ -33,6 +59,22 @@ static void jsonb_in_array_end(void *pstate);
static void jsonb_in_object_field_start(void *pstate, char *fname, bool isnull);
static void jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal);
static void jsonb_in_scalar(void *pstate, char *token, JsonTokenType tokentype);
+static void jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid);
+static void composite_to_jsonb(Datum composite, JsonbInState *result);
+static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims,
+ Datum *vals, bool *nulls, int *valcount,
+ JsonbTypeCategory tcategory, Oid outfuncoid);
+static void array_to_jsonb_internal(Datum array, JsonbInState *result);
+static void jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid);
+static void datum_to_jsonb(Datum val, bool is_null, JsonbInState *result,
+ JsonbTypeCategory tcategory, Oid outfuncoid,
+ bool key_scalar);
+static void add_jsonb(Datum val, bool is_null, JsonbInState *result,
+ Oid val_type, bool key_scalar);
/*
* jsonb type input function
@@ -462,3 +504,1282 @@ JsonbToCString(StringInfo out, JsonbContainer *in, int estimated_len)
return out-data;
}
+
+
+/*
+ * Determine how we want to render values of a given type in datum_to_jsonb.
+ *
+ * Given the datatype OID, return its JsonbTypeCategory, as well as the type's
+ * output function OID. If the returned category is JSONBTYPE_CAST, we
+ * return the OID of the type-JSON cast function instead.
+ */
+static void
+jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid)
+{
+ bool typisvarlena;
+
+ /* Look through any domain */
+ typoid = getBaseType(typoid);
+
+ /* We'll usually need to return the type output function */
+ getTypeOutputInfo(typoid, outfuncoid, typisvarlena);
+
+ /* Check for known types */
+ switch (typoid)
+ {
+ case BOOLOID:
+ *tcategory = JSONBTYPE_BOOL;
+ break;
+
+ case INT2OID:
+ case INT4OID:
+ case INT8OID:
+ case FLOAT4OID:
+ case FLOAT8OID:
+ case NUMERICOID:
+ *tcategory = JSONBTYPE_NUMERIC;
+ break;
+
+ case TIMESTAMPOID:
+ *tcategory = JSONBTYPE_TIMESTAMP;
+ break;
+
+ case TIMESTAMPTZOID:
+ *tcategory = JSONBTYPE_TIMESTAMPTZ;
+ break;
+
+ case JSONBOID:
+ *tcategory = JSONBTYPE_JSONB;
+ break;
+
+ case JSONOID:
+ *tcategory = JSONBTYPE_JSON;
+ break;
+
+ default:
+ /* Check for arrays and composites */
+ if (OidIsValid(get_element_type(typoid)))
+*tcategory = JSONBTYPE_ARRAY;
+ else if (type_is_rowtype(typoid))
+*tcategory = JSONBTYPE_COMPOSITE;
+ else
+ {
+/* It's probably the general case ... */
+*tcategory = JSONBTYPE_OTHER;
+
+/*
+ * but let's look for a cast to json or jsonb, if it's not
+ * built-in
+ */
+
Re: [HACKERS] JsonbValue to Jsonb conversion
On 10/13/2014 06:41 AM, Pavel Stehule wrote: Hi I am working on review of this patch. The patch attached to the message you are replying to was never intended to be reviewed. It was only given by way of illustration of a technique. The original patch to be reviewed is on the message http://www.postgresql.org/message-id/5425d277.4030...@dunslane.net as shown on the commitfest app. I have just submitted a revised patch to fix the compiler warnings you complained of, at http://www.postgresql.org/message-id/543bd598.4020...@dunslane.net I have not found any segfaults in the regression tests. And please don't top-post on the PostgreSQL lists. 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] split builtins.h to quote.h
Michael Paquier michael.paqu...@gmail.com writes: On Mon, Oct 13, 2014 at 10:04 PM, Robert Haas robertmh...@gmail.com wrote: No significant advantage will be gained by splitting it out and then #including it; nobody's really going to fix their module builds until they actually break. On the general substance of the issue, our usual convention is that src/backend/X/Y/Z.c has its prototypes in src/include/X/Z.h. If this proposal moves us closer to that, I'm OK with enduring the module breakage that will result. If, on the other hand, it in moves us further away from that, then I'm against it. That's a 2/2 tie then AFAIK: Noah and Stephen express concerns about the breakage, you and I would be fine with a clear breakage to make code more organized (correct me if you don't feel this way). FWIW, we break code *all the time* in the direction of requiring new #include's. I think the argument that this particular case should be sacrosanct is pretty thin. If we were back-patching then the considerations would be different --- but as long as it's 9.5 material I have no problem with it. What I find strange about the actual patch is that it moves some but not all of the prototypes for the stuff that ends up in quote.c into quote.h. That doesn't seem right. Are you referring to the Datum quote_*(PG_FUNCTION_ARGS) that are still let in builtins.h? That was let on purpose to let all the SQL functions within builtins.h but I'd be happy to move everything to quote.h to make the separation clearer. I agree with Robert on this one. builtins.h is really just a refuge for declaring SQL-level functions that have no other natural home. 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] split builtins.h to quote.h
I wrote: Michael Paquier michael.paqu...@gmail.com writes: Are you referring to the Datum quote_*(PG_FUNCTION_ARGS) that are still let in builtins.h? That was let on purpose to let all the SQL functions within builtins.h but I'd be happy to move everything to quote.h to make the separation clearer. I agree with Robert on this one. builtins.h is really just a refuge for declaring SQL-level functions that have no other natural home. Actually, on second thought I'm not so sure. What we've done in other modules is to put SQL function declarations into builtins.h rather than a module-specific header, because otherwise we'd have had to #include fmgr.h in the module-specific header, and that did not seem like a win. If there is some independent reason why quote.h needs to include fmgr.h then we may as well move the SQL functions there too; but if not, I'd just as soon keep down the amount of header inclusion needed. 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] split builtins.h to quote.h
On Mon, Oct 13, 2014 at 9:24 AM, Michael Paquier michael.paqu...@gmail.com wrote: That's a 2/2 tie then AFAIK: Noah and Stephen express concerns about the breakage, you and I would be fine with a clear breakage to make code more organized (correct me if you don't feel this way). Well, I think that the long-standing agglomeration of prototypes from many header files into builtins.h is kind of a wart. But I also think that it's a justified wart, to the extent that we wish to avoid creating many header files with only a tiny handful of prototypes. So I'm not in a tremendous hurry to change it, but I'm OK with changing it if other people feel there's a benefit. The main reason to separate out quote.h, AIUI, is that unlike a lot of the stuff in builtins.h, those functions are actually called from quite a few places. What I find strange about the actual patch is that it moves some but not all of the prototypes for the stuff that ends up in quote.c into quote.h. That doesn't seem right. Are you referring to the Datum quote_*(PG_FUNCTION_ARGS) that are still let in builtins.h? That was let on purpose to let all the SQL functions within builtins.h but I'd be happy to move everything to quote.h to make the separation clearer. IMHO, putting some prototypes for a .c file in one header and others in another header is going to make it significantly harder to figure out which files you need to #include when. Keeping a simple rule there seems essential to me. -- 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] JsonbValue to Jsonb conversion
2014-10-13 15:45 GMT+02:00 Andrew Dunstan and...@dunslane.net: On 10/13/2014 06:41 AM, Pavel Stehule wrote: Hi I am working on review of this patch. The patch attached to the message you are replying to was never intended to be reviewed. It was only given by way of illustration of a technique. The original patch to be reviewed is on the message http://www.postgresql.org/message-id/5425d277.4030...@dunslane.net as shown on the commitfest app. I have just submitted a revised patch to fix the compiler warnings you complained of, at http://www.postgresql.org/ message-id/543bd598.4020...@dunslane.net I have not found any segfaults in the regression tests. I checked this last version - warning is out, but SIGFAULT on jsonb test is there .. I rechecked it with clang compiler, but result is same I try to search why And please don't top-post on the PostgreSQL lists. I am sorry Regards Pavel cheers andrew
Re: [HACKERS] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On Sun, Oct 12, 2014 at 5:36 AM, Andres Freund and...@2ndquadrant.com wrote: On 2014-10-11 20:33:57 -0400, Bruce Momjian wrote: On Tue, Aug 12, 2014 at 07:08:06PM -0400, Robert Haas wrote: On Tue, Aug 12, 2014 at 12:59 PM, Andres Freund and...@2ndquadrant.com wrote: On 2014-08-12 09:42:30 -0700, Sean Chittenden wrote: One of the patches that I've been sitting on and am derelict in punting upstream is the attached mmap(2) flags patch for the BSDs. Is there any chance this can be squeezed in to the PostreSQL 9.4 release? The patch is trivial in size and is used to add one flag to mmap(2) calls in dsm_impl.c. Alan Cox (FreeBSD alc, not Linux) and I went back and forth regarding PostgreSQL's use of mmap(2) and determined that the following is correct and will prevent a likely performance regression in PostgreSQL 9.4. In PostgreSQL 9.3, all mmap(2) calls were called with the flags MAP_ANON | MAP_SHARED, whereas in PostgreSQL 9.4 this is not the case. The performancewise important call to mmap will still use that set of flags, no? That's the one backing shared_buffers. The mmap backend for *dynamic* shared memory (aka dsm) is *NOT* supposed to be used on common platforms. Both posix and sysv shared memory will be used before falling back to the mmap() backend. Hmm, yeah. This might still be a good thing to do (because what do we lose?) but it shouldn't really be an issue in practice. Is there a reason this was not applied? IIRC, as pointed out above, it's primarily based on a misunderstanding about when mmap is used for in dsm. I.e. that it's essentially just a fallback/toy implementation and that posix or sysv should rather be used. Perhaps, but I still see no reason not to apply it. It may not help many people, but it won't hurt anything, either. So why not? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal : REINDEX SCHEMA
On Sun, Oct 12, 2014 at 1:27 PM, Stephen Frost sfr...@snowman.net wrote: * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Sawada Masahiko wrote: Attached WIP patch adds new syntax REINEX SCHEMA which does reindexing all table of specified schema. There are syntax dose reindexing specified index, per table and per database, but we can not do reindexing per schema for now. It seems doubtful that there really is much use for this feature, but if there is, I think a better syntax precedent is the new ALTER TABLE ALL IN TABLESPACE thingy, rather than your proposed REINDEX SCHEMA. Something like REINDEX TABLE ALL IN SCHEMA perhaps. Yeah, I tend to agree that we should be looking at the 'ALL IN TABLESPACE' and 'ALL IN SCHEMA' type of commands to keep things consistent. This might be an alternative for the vacuum / analyze / reindex database commands also.. Urgh. I don't have a problem with that syntax in general, but it clashes pretty awfully with what we're already doing for REINDEX otherwise. -- 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] JsonbValue to Jsonb conversion
Pavel Stehule pavel.steh...@gmail.com writes: I checked this last version - warning is out, but SIGFAULT on jsonb test is there .. I rechecked it with clang compiler, but result is same Stack trace please? 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] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On 2014-10-13 10:15:29 -0400, Robert Haas wrote: On Sun, Oct 12, 2014 at 5:36 AM, Andres Freund and...@2ndquadrant.com wrote: IIRC, as pointed out above, it's primarily based on a misunderstanding about when mmap is used for in dsm. I.e. that it's essentially just a fallback/toy implementation and that posix or sysv should rather be used. Perhaps, but I still see no reason not to apply it. It may not help many people, but it won't hurt anything, either. So why not? More complicated, less tested code. For no practial benefit, it'll still be slower than posix shm if there's any memmory pressure. But if you want to apply it, go ahead, I won't cry louder than this email. I still think the mmap dsm implementation is a bad idea. We shouldn't put additional effort into it. If anything we should remove it. Greetings, Andres Freund -- Andres Freund 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] JsonbValue to Jsonb conversion
2014-10-13 16:19 GMT+02:00 Tom Lane t...@sss.pgh.pa.us:
Pavel Stehule pavel.steh...@gmail.com writes:
I checked this last version - warning is out, but SIGFAULT on jsonb test
is
there .. I rechecked it with clang compiler, but result is same
Stack trace please?
(gdb) bt
#0 0x0072 in ?? ()
#1 0x0087d598 in parse_array_element (lex=0x2880118,
sem=0x7fffb4f02508) at json.c:461
#2 0x00878da7 in parse_array (lex=0x2880118, sem=0x7fffb4f02508)
at json.c:505
#3 0x0087d837 in parse_object_field (lex=0x2880118,
sem=0x7fffb4f02508) at json.c:391
#4 0x00878cb2 in parse_object (lex=0x2880118, sem=0x7fffb4f02508)
at json.c:432
#5 0x0087831c in pg_parse_json (lex=0x2880118, sem=0x7fffb4f02508)
at json.c:297
#6 0x0087f484 in datum_to_jsonb (val=42202912, is_null=0 '\000',
result=0x7fffb4f02800,
tcategory=JSONBTYPE_JSON, outfuncoid=322, key_scalar=0 '\000') at
jsonb.c:789
#7 0x0087fce7 in add_jsonb (val=42202912, is_null=0 '\000',
result=0x7fffb4f02800, val_type=114,
key_scalar=0 '\000') at jsonb.c:1050
#8 0x0087fbcc in jsonb_build_object (fcinfo=0x287e2c0) at
jsonb.c:1155
#9 0x0066d179 in ExecMakeFunctionResultNoSets (fcache=0x287e250,
econtext=0x287e060, isNull=0x287eca8 ,
isDone=0x287edc0) at execQual.c:1992
#10 0x0066776f in ExecEvalFunc (fcache=0x287e250,
econtext=0x287e060, isNull=0x287eca8 , isDone=0x287edc0)
at execQual.c:2383
#11 0x0066c3bb in ExecTargetList (targetlist=0x287ed90,
econtext=0x287e060, values=0x287ec90,
isnull=0x287eca8 , itemIsDone=0x287edc0, isDone=0x7fffb4f02aac) at
execQual.c:5265
#12 0x0066c2c2 in ExecProject (projInfo=0x287ecc0,
isDone=0x7fffb4f02aac) at execQual.c:5480
#13 0x00689ceb in ExecResult (node=0x287df50) at nodeResult.c:155
#14 0x00661987 in ExecProcNode (node=0x287df50) at
execProcnode.c:373
#15 0x0065dd46 in ExecutePlan (estate=0x287de40,
planstate=0x287df50, operation=CMD_SELECT,
sendTuples=1 '\001', numberTuples=0, direction=ForwardScanDirection,
dest=0x283fa00) at execMain.c:1481
#16 0x0065dc70 in standard_ExecutorRun (queryDesc=0x2809d50,
direction=ForwardScanDirection, count=0)
at execMain.c:308
#17 0x0065db3f in ExecutorRun (queryDesc=0x2809d50,
direction=ForwardScanDirection, count=0)
at execMain.c:256
#18 0x007ec70c in PortalRunSelect (portal=0x2807bc0, forward=1
'\001', count=0, dest=0x283fa00)
at pquery.c:946
#19 0x007ec229 in PortalRun (portal=0x2807bc0,
count=9223372036854775807, isTopLevel=1 '\001',
dest=0x283fa00, altdest=0x283fa00, completionTag=0x7fffb4f02ec0 ) at
pquery.c:790
#20 0x007e7f7c in exec_simple_query (
query_string=0x283e1a0 SELECT jsonb_build_object('e',json '{\x\: 3,
\y\: [1,2,3]}');) at postgres.c:1045
#21 0x007e72cb in PostgresMain (argc=1, argv=0x27e5838,
dbname=0x27e56e8 postgres,
---Type return to continue, or q return to quit---q
username=0x27e56d0 paveQuit
Regards
Pavel
regards, tom lane
Re: [HACKERS] JsonbValue to Jsonb conversion
Hi
A JsonSemAction sem is not well initialized
a array_element_start is not initialized and enforces sigfault on my comp
*** ./utils/adt/jsonb.c.orig2014-10-13 16:37:00.479708142 +0200
--- ./utils/adt/jsonb.c2014-10-13 16:36:33.704650644 +0200
***
*** 786,791
--- 786,793
sem.scalar = jsonb_in_scalar;
sem.object_field_start = jsonb_in_object_field_start;
+ sem.array_element_start = NULL;
+
pg_parse_json(lex, sem);
}
I am not sure, if this fix is valid, but all tests are passed now
Regards
Pavel
2014-10-13 16:21 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
2014-10-13 16:19 GMT+02:00 Tom Lane t...@sss.pgh.pa.us:
Pavel Stehule pavel.steh...@gmail.com writes:
I checked this last version - warning is out, but SIGFAULT on jsonb
test is
there .. I rechecked it with clang compiler, but result is same
Stack trace please?
(gdb) bt
#0 0x0072 in ?? ()
#1 0x0087d598 in parse_array_element (lex=0x2880118,
sem=0x7fffb4f02508) at json.c:461
#2 0x00878da7 in parse_array (lex=0x2880118, sem=0x7fffb4f02508)
at json.c:505
#3 0x0087d837 in parse_object_field (lex=0x2880118,
sem=0x7fffb4f02508) at json.c:391
#4 0x00878cb2 in parse_object (lex=0x2880118, sem=0x7fffb4f02508)
at json.c:432
#5 0x0087831c in pg_parse_json (lex=0x2880118,
sem=0x7fffb4f02508) at json.c:297
#6 0x0087f484 in datum_to_jsonb (val=42202912, is_null=0 '\000',
result=0x7fffb4f02800,
tcategory=JSONBTYPE_JSON, outfuncoid=322, key_scalar=0 '\000') at
jsonb.c:789
#7 0x0087fce7 in add_jsonb (val=42202912, is_null=0 '\000',
result=0x7fffb4f02800, val_type=114,
key_scalar=0 '\000') at jsonb.c:1050
#8 0x0087fbcc in jsonb_build_object (fcinfo=0x287e2c0) at
jsonb.c:1155
#9 0x0066d179 in ExecMakeFunctionResultNoSets (fcache=0x287e250,
econtext=0x287e060, isNull=0x287eca8 ,
isDone=0x287edc0) at execQual.c:1992
#10 0x0066776f in ExecEvalFunc (fcache=0x287e250,
econtext=0x287e060, isNull=0x287eca8 , isDone=0x287edc0)
at execQual.c:2383
#11 0x0066c3bb in ExecTargetList (targetlist=0x287ed90,
econtext=0x287e060, values=0x287ec90,
isnull=0x287eca8 , itemIsDone=0x287edc0, isDone=0x7fffb4f02aac) at
execQual.c:5265
#12 0x0066c2c2 in ExecProject (projInfo=0x287ecc0,
isDone=0x7fffb4f02aac) at execQual.c:5480
#13 0x00689ceb in ExecResult (node=0x287df50) at nodeResult.c:155
#14 0x00661987 in ExecProcNode (node=0x287df50) at
execProcnode.c:373
#15 0x0065dd46 in ExecutePlan (estate=0x287de40,
planstate=0x287df50, operation=CMD_SELECT,
sendTuples=1 '\001', numberTuples=0, direction=ForwardScanDirection,
dest=0x283fa00) at execMain.c:1481
#16 0x0065dc70 in standard_ExecutorRun (queryDesc=0x2809d50,
direction=ForwardScanDirection, count=0)
at execMain.c:308
#17 0x0065db3f in ExecutorRun (queryDesc=0x2809d50,
direction=ForwardScanDirection, count=0)
at execMain.c:256
#18 0x007ec70c in PortalRunSelect (portal=0x2807bc0, forward=1
'\001', count=0, dest=0x283fa00)
at pquery.c:946
#19 0x007ec229 in PortalRun (portal=0x2807bc0,
count=9223372036854775807, isTopLevel=1 '\001',
dest=0x283fa00, altdest=0x283fa00, completionTag=0x7fffb4f02ec0 ) at
pquery.c:790
#20 0x007e7f7c in exec_simple_query (
query_string=0x283e1a0 SELECT jsonb_build_object('e',json '{\x\: 3,
\y\: [1,2,3]}');) at postgres.c:1045
#21 0x007e72cb in PostgresMain (argc=1, argv=0x27e5838,
dbname=0x27e56e8 postgres,
---Type return to continue, or q return to quit---q
username=0x27e56d0 paveQuit
Regards
Pavel
regards, tom lane
Re: [HACKERS] UPSERT wiki page, and SQL MERGE syntax
On Fri, Oct 10, 2014 at 4:41 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Oct 10, 2014 at 12:09 PM, Robert Haas robertmh...@gmail.com wrote: I think what's realistic here is that the patch isn't going to get committed the way you have it today, because nobody else likes that design. That may be harsh, but I think it's accurate. I do think that's harsh, because it's unnecessary: I am looking for the best design. If you want to propose alternatives, great, but there is a reason why I've done things that way, and that should be acknowledged. I too think naming the unique index is ugly as sin, and have said as much multiple times. We're almost on the same page here. Come on. You've had the syntax that way for a while, multiple people have objected to it, and it didn't get changed. When people renewed their objections, you said that they were not having a realistic conversation. I'm tired of getting critiqued because there's some fine point of one of the scores of emails you've sent on this topic that you feel I haven't adequately appreciated. I would like to avoid getting into a flame-war here where we spend a lot of time arguing about who should have said what in exactly what way, but I think it is fair to say that your emails have come across to me, and to a number of other people here, as digging in your heels and refusing to budge. I would go so far as to say that said perception is the primary reason why this patch is making so little progress, far outstripping whatever the actual technical issues are. Whatever the actual behavior of your patch is today, it seems to me that the conflict is, fundamentally, over a set of column values, NOT over a particular index. The index is simply a mechanism for noticing that conflict. I think that this is the kernel of our disagreement. The index is not simply a mechanism for noticing that conflict. The user had better have one unique index in mind to provoke taking the alternative path in the event of a would-be unique violation. Clearly it doesn't matter much in this particular example. But what if there were two partial unique indexes, that were actually distinct, but only in terms of the constant appearing in their predicate? And what about having that changed by a before insert row-level trigger? Are we to defer deciding which unique index to use at the last possible minute? I don't immediately see why this would cause a problem. IIUC, the scenario we're talking about is: create table foo (a int, b int); create unique index foo1 on foo (a) where b = 1; create unique index foo2 on foo (a) where b = 2; If the user issues INSERT .. ON DUPLICATE (a) UPDATE, we'll fire before-insert triggers and then inspect the tuple to be inserted. If b is neither 1 nor 2, then we'll fail with an error saying that we can't support ON DUPLICATE without a relevant index to enforce uniqueness. (This can presumably re-use the same error message that would have popped out if the user done ON DUPLICATE (b), which is altogether un-indexed.) But if b is 1 or 2, then we'll search the matching index for a conflicting tuple; finding none, we'll insert; finding one, we'll do the update as per the user's instructions. As always with this stuff, the devil is in the details. If we work out the details, then I can come up with something that's acceptable to everyone. Would you be okay with this never working with partial unique indexes? That gives me something to work with. If it can't be made to work with them in a reasonable fashion, I would probably be OK with that, but so far I see no reason for such pessimism. Also, how about making the SET clause optional, with the semantics that we just update all of the fields for which a value is explicitly specified: INSERT INTO overwrite_with_abandon (key, value) VALUES (42, 'meaning of life') ON DUPLICATE (key) UPDATE; Your syntax allows the exact same thing; it simply require the user to be more verbose in order to get that behavior. If we think that people wanting that behavior will be rare, then it's fine to require them to spell it out when they want it. If we think it will be the overwhelming common application of this feature, and I do, then making people spell it out when we could just as well infer it is pointless. Did you consider my example? I think that people will like this idea, too - that clearly isn't the only consideration, though. As you say, it would be very easy to implement this. However, IMV, we shouldn't, because it is hazardous. MySQL doesn't allow this, and they tend to find expedients like this useful. I'm considering your points in this area as well as I can, but as far as I can tell you haven't actually set what's bad about it, just that it might be hazardous in some way that you don't appear to have specified, and that MySQL doesn't allow it. I am untroubled by the latter point; it is not our goal to confine our implementation to a subset of
Re: [HACKERS] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
Perhaps, but I still see no reason not to apply it. It may not help many people, but it won't hurt anything, either. So why not? More complicated, less tested code. For no practial benefit, it'll still be slower than posix shm if there's any memmory pressure. But if you want to apply it, go ahead, I won't cry louder than this email. I still think the mmap dsm implementation is a bad idea. We shouldn't put additional effort into it. If anything we should remove it. While you're not wrong in that use of mmap(2) here is potentially a bad idea, much of that is mitigated through the correct use of flags to mmap(2) (i.e. prevent mmap(2) pages from hooking in to the syncer). In the same breath, it would also be nice if the following were committed: --- src/template/freebsd.orig 2014-05-26 23:54:53.854165855 +0300 +++ src/template/freebsd2014-05-26 23:55:12.307880900 +0300 @@ -3,3 +3,4 @@ case $host_cpu in alpha*) CFLAGS=-O;; # alpha has problems with -O2 esac +USE_NAMED_POSIX_SEMAPHORES=1 -sc -- Sean Chittenden s...@chittenden.org -- 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] JsonbValue to Jsonb conversion
2014-10-13 15:45 GMT+02:00 Andrew Dunstan and...@dunslane.net:
On 10/13/2014 06:41 AM, Pavel Stehule wrote:
Hi
I am working on review of this patch.
The patch attached to the message you are replying to was never intended
to be reviewed. It was only given by way of illustration of a technique.
The original patch to be reviewed is on the message
http://www.postgresql.org/message-id/5425d277.4030...@dunslane.net as
shown on the commitfest app. I have just submitted a revised patch to fix
the compiler warnings you complained of, at http://www.postgresql.org/
message-id/543bd598.4020...@dunslane.net I have not found any segfaults
in the regression tests.
And please don't top-post on the PostgreSQL lists.
Attached small fix of uninitialized local variable
Regards
Pavel
cheers
andrew
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
new file mode 100644
index 9beebb3..c9b84f8
*** a/src/backend/utils/adt/jsonb.c
--- b/src/backend/utils/adt/jsonb.c
***
*** 12,22
--- 12,31
*/
#include postgres.h
+ #include miscadmin.h
+ #include access/htup_details.h
+ #include access/transam.h
+ #include catalog/pg_cast.h
+ #include catalog/pg_type.h
#include libpq/pqformat.h
#include utils/builtins.h
+ #include utils/datetime.h
+ #include utils/lsyscache.h
#include utils/json.h
#include utils/jsonapi.h
#include utils/jsonb.h
+ #include utils/syscache.h
+ #include utils/typcache.h
typedef struct JsonbInState
{
*** typedef struct JsonbInState
*** 24,29
--- 33,55
JsonbValue *res;
} JsonbInState;
+ /* unlike with json categories, we need to treat json and jsonb differently */
+ typedef enum /* type categories for datum_to_jsonb */
+ {
+ JSONBTYPE_NULL,/* null, so we didn't bother to identify */
+ JSONBTYPE_BOOL,/* boolean (built-in types only) */
+ JSONBTYPE_NUMERIC, /* numeric (ditto) */
+ JSONBTYPE_TIMESTAMP, /* we use special formatting for timestamp */
+ JSONBTYPE_TIMESTAMPTZ, /* ... and timestamptz */
+ JSONBTYPE_JSON,/* JSON */
+ JSONBTYPE_JSONB, /* JSONB */
+ JSONBTYPE_ARRAY, /* array */
+ JSONBTYPE_COMPOSITE, /* composite */
+ JSONBTYPE_JSONCAST, /* something with an explicit cast to JSON */
+ JSONBTYPE_JSONBCAST, /* something with an explicit cast to JSONB */
+ JSONBTYPE_OTHER/* all else */
+ } JsonbTypeCategory;
+
static inline Datum jsonb_from_cstring(char *json, int len);
static size_t checkStringLen(size_t len);
static void jsonb_in_object_start(void *pstate);
*** static void jsonb_in_array_end(void *pst
*** 33,38
--- 59,80
static void jsonb_in_object_field_start(void *pstate, char *fname, bool isnull);
static void jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal);
static void jsonb_in_scalar(void *pstate, char *token, JsonTokenType tokentype);
+ static void jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid);
+ static void composite_to_jsonb(Datum composite, JsonbInState *result);
+ static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims,
+ Datum *vals, bool *nulls, int *valcount,
+ JsonbTypeCategory tcategory, Oid outfuncoid);
+ static void array_to_jsonb_internal(Datum array, JsonbInState *result);
+ static void jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid);
+ static void datum_to_jsonb(Datum val, bool is_null, JsonbInState *result,
+ JsonbTypeCategory tcategory, Oid outfuncoid,
+ bool key_scalar);
+ static void add_jsonb(Datum val, bool is_null, JsonbInState *result,
+ Oid val_type, bool key_scalar);
/*
* jsonb type input function
*** JsonbToCString(StringInfo out, JsonbCont
*** 462,464
--- 504,1786
return out-data;
}
+
+
+ /*
+ * Determine how we want to render values of a given type in datum_to_jsonb.
+ *
+ * Given the datatype OID, return its JsonbTypeCategory, as well as the type's
+ * output function OID. If the returned category is JSONBTYPE_CAST, we
+ * return the OID of the type-JSON cast function instead.
+ */
+ static void
+ jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid)
+ {
+ bool typisvarlena;
+
+ /* Look through any domain */
+ typoid = getBaseType(typoid);
+
+ /* We'll usually need to return the type output function */
+ getTypeOutputInfo(typoid, outfuncoid, typisvarlena);
+
+ /* Check for known types */
+ switch (typoid)
+ {
+ case BOOLOID:
+ *tcategory = JSONBTYPE_BOOL;
+ break;
+
+ case INT2OID:
+ case INT4OID:
+ case INT8OID:
+ case FLOAT4OID:
+ case FLOAT8OID:
+ case NUMERICOID:
+ *tcategory = JSONBTYPE_NUMERIC;
+ break;
+
+ case TIMESTAMPOID:
+ *tcategory = JSONBTYPE_TIMESTAMP;
+ break;
+
+ case TIMESTAMPTZOID:
+ *tcategory = JSONBTYPE_TIMESTAMPTZ;
+ break;
+
+
Re: [HACKERS] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On 2014-10-13 07:49:44 -0700, Sean Chittenden wrote: Perhaps, but I still see no reason not to apply it. It may not help many people, but it won't hurt anything, either. So why not? More complicated, less tested code. For no practial benefit, it'll still be slower than posix shm if there's any memmory pressure. But if you want to apply it, go ahead, I won't cry louder than this email. I still think the mmap dsm implementation is a bad idea. We shouldn't put additional effort into it. If anything we should remove it. While you're not wrong in that use of mmap(2) here is potentially a bad idea, much of that is mitigated through the correct use of flags to mmap(2) (i.e. prevent mmap(2) pages from hooking in to the syncer). In the same breath, it would also be nice if the following were committed: Unless I'm mistaken the pages will still be written back to disk (and not just swap, the actual backing file) if there's memory pressure, no? --- src/template/freebsd.orig 2014-05-26 23:54:53.854165855 +0300 +++ src/template/freebsd2014-05-26 23:55:12.307880900 +0300 @@ -3,3 +3,4 @@ case $host_cpu in alpha*) CFLAGS=-O;; # alpha has problems with -O2 esac +USE_NAMED_POSIX_SEMAPHORES=1 If so, that should be a separate change. But why? Greetings, Andres Freund -- Andres Freund 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] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
Sean Chittenden s...@chittenden.org writes: In the same breath, it would also be nice if the following were committed: [ use named POSIX semaphores on FreeBSD ] Really? Why? According to the notes in our code, named POSIX semaphores are the least attractive of the three Unixoid semaphore APIs we support, because they require eating a file descriptor per backend per max_connection slot. That's a lot of FDs in any large configuration. FreeBSD's support for SysV semaphores would have to be pretty darn awful to make me think this was a good change, and I've not heard complaints in that direction before. If you meant to propose using *unnamed* POSIX semaphores, that might be a reasonable change, but it would still need some supporting evidence. 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] psql output change in 9.4
Bruce Momjian br...@momjian.us writes: On Sun, Oct 12, 2014 at 12:17:31AM -0400, Peter Eisentraut wrote: Patch attached. I think this would be for 9.5 only, at this point. Funny, I was *just* working on that, too. I propose a patch that reverts the output to how it was in 9.3 (without anything in parentheses), and implements the output of \pset without any arguments separately, thus: Agreed. Works for me, too. If we are reverting to 9.3's output in the base case, I think this *does* need to get back-patched into 9.4. 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] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On Mon, Oct 13, 2014 at 04:19:39PM +0200, Andres Freund wrote: On 2014-10-13 10:15:29 -0400, Robert Haas wrote: On Sun, Oct 12, 2014 at 5:36 AM, Andres Freund and...@2ndquadrant.com wrote: IIRC, as pointed out above, it's primarily based on a misunderstanding about when mmap is used for in dsm. I.e. that it's essentially just a fallback/toy implementation and that posix or sysv should rather be used. Perhaps, but I still see no reason not to apply it. It may not help many people, but it won't hurt anything, either. So why not? More complicated, less tested code. For no practical benefit, it'll still be slower than posix shm if there's any memmory pressure. But if you want to apply it, go ahead, I won't cry louder than this email. I still think the mmap dsm implementation is a bad idea. We shouldn't put additional effort into it. If anything we should remove it. If we have it, we should improve it, or remove it. We might want to use this code for something else in the future, so it should be improved where feasible. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On 2014-10-13 11:18:26 -0400, Bruce Momjian wrote: On Mon, Oct 13, 2014 at 04:19:39PM +0200, Andres Freund wrote: On 2014-10-13 10:15:29 -0400, Robert Haas wrote: On Sun, Oct 12, 2014 at 5:36 AM, Andres Freund and...@2ndquadrant.com wrote: IIRC, as pointed out above, it's primarily based on a misunderstanding about when mmap is used for in dsm. I.e. that it's essentially just a fallback/toy implementation and that posix or sysv should rather be used. Perhaps, but I still see no reason not to apply it. It may not help many people, but it won't hurt anything, either. So why not? More complicated, less tested code. For no practical benefit, it'll still be slower than posix shm if there's any memmory pressure. But if you want to apply it, go ahead, I won't cry louder than this email. I still think the mmap dsm implementation is a bad idea. We shouldn't put additional effort into it. If anything we should remove it. If we have it, we should improve it, or remove it. We might want to use this code for something else in the future, so it should be improved where feasible. Meh. We don't put in effort into code that doesn't matter just because it might get used elsewhere some day. By that argument we'd need to performance optimize a lot of code. And actually, using that code somewhere else is more of a counter indication than a pro argument. MAP_NOSYNC isn't a general purpose flag. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_get_indexdef() doesn't quote string reloptions
Hi all! I've been working on implementing a custom index using the Index Access Method API and have the need for custom reloptions that are complex strings (ie, also contain non-alphaumerics). pg_get_indexdef() and pg_dump don't quote the reloption values, making a restore (or cut-n-paste of the pg_get_indexdef() output) impossible if the reloption value contains non-alphanumerics. For example, the statement: # CREATE INDEX idxfoo ON table USING myindex (col) WITH (option = 'some complex string'); cannot be restored as it gets rewritten as: CREATE INDEX idxfoo ON table USING myindex (col) WITH (option = some complex string); (note the lack of quotes around the option value) Looks like (at least) ruleutils.c:flatten_reloptions() needs to be smarter. eric PROPRIETARY AND COMPANY CONFIDENTIAL COMMUNICATIONS The information contained in this communication is intended only for the use of the addressee. Any other use is strictly prohibited. Please notify the sender if you have received this message in error. This communication is protected by applicable legal privileges and is company confidential. -- 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] jsonb generator functions
On 10/13/2014 09:37 AM, Andrew Dunstan wrote:
On 09/26/2014 04:54 PM, Andrew Dunstan wrote:
Here is a patch for the generator and aggregate functions for jsonb
that we didn't manage to get done in time for 9.4. They are all
equivalents of the similarly names json functions. Included are
to_jsonb
jsonb_build_object
jsonb_build_array
jsonb_object
jsonb_agg
jsonb_object_agg
Still to come: documentation.
Adding to the next commitfest.
Revised patch to fix compiler warnings.
And again, initializing an incompletely initialized variable, as found
by Pavel Stehule.
cheers
andrew
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 2fd87fc..33a19be 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -12,11 +12,20 @@
*/
#include postgres.h
+#include miscadmin.h
+#include access/htup_details.h
+#include access/transam.h
+#include catalog/pg_cast.h
+#include catalog/pg_type.h
#include libpq/pqformat.h
#include utils/builtins.h
+#include utils/datetime.h
+#include utils/lsyscache.h
#include utils/json.h
#include utils/jsonapi.h
#include utils/jsonb.h
+#include utils/syscache.h
+#include utils/typcache.h
typedef struct JsonbInState
{
@@ -24,6 +33,23 @@ typedef struct JsonbInState
JsonbValue *res;
} JsonbInState;
+/* unlike with json categories, we need to treat json and jsonb differently */
+typedef enum /* type categories for datum_to_jsonb */
+{
+ JSONBTYPE_NULL,/* null, so we didn't bother to identify */
+ JSONBTYPE_BOOL,/* boolean (built-in types only) */
+ JSONBTYPE_NUMERIC, /* numeric (ditto) */
+ JSONBTYPE_TIMESTAMP, /* we use special formatting for timestamp */
+ JSONBTYPE_TIMESTAMPTZ, /* ... and timestamptz */
+ JSONBTYPE_JSON,/* JSON */
+ JSONBTYPE_JSONB, /* JSONB */
+ JSONBTYPE_ARRAY, /* array */
+ JSONBTYPE_COMPOSITE, /* composite */
+ JSONBTYPE_JSONCAST, /* something with an explicit cast to JSON */
+ JSONBTYPE_JSONBCAST, /* something with an explicit cast to JSONB */
+ JSONBTYPE_OTHER/* all else */
+} JsonbTypeCategory;
+
static inline Datum jsonb_from_cstring(char *json, int len);
static size_t checkStringLen(size_t len);
static void jsonb_in_object_start(void *pstate);
@@ -33,6 +59,22 @@ static void jsonb_in_array_end(void *pstate);
static void jsonb_in_object_field_start(void *pstate, char *fname, bool isnull);
static void jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal);
static void jsonb_in_scalar(void *pstate, char *token, JsonTokenType tokentype);
+static void jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid);
+static void composite_to_jsonb(Datum composite, JsonbInState *result);
+static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims,
+ Datum *vals, bool *nulls, int *valcount,
+ JsonbTypeCategory tcategory, Oid outfuncoid);
+static void array_to_jsonb_internal(Datum array, JsonbInState *result);
+static void jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid);
+static void datum_to_jsonb(Datum val, bool is_null, JsonbInState *result,
+ JsonbTypeCategory tcategory, Oid outfuncoid,
+ bool key_scalar);
+static void add_jsonb(Datum val, bool is_null, JsonbInState *result,
+ Oid val_type, bool key_scalar);
/*
* jsonb type input function
@@ -462,3 +504,1284 @@ JsonbToCString(StringInfo out, JsonbContainer *in, int estimated_len)
return out-data;
}
+
+
+/*
+ * Determine how we want to render values of a given type in datum_to_jsonb.
+ *
+ * Given the datatype OID, return its JsonbTypeCategory, as well as the type's
+ * output function OID. If the returned category is JSONBTYPE_CAST, we
+ * return the OID of the type-JSON cast function instead.
+ */
+static void
+jsonb_categorize_type(Oid typoid,
+ JsonbTypeCategory * tcategory,
+ Oid *outfuncoid)
+{
+ bool typisvarlena;
+
+ /* Look through any domain */
+ typoid = getBaseType(typoid);
+
+ /* We'll usually need to return the type output function */
+ getTypeOutputInfo(typoid, outfuncoid, typisvarlena);
+
+ /* Check for known types */
+ switch (typoid)
+ {
+ case BOOLOID:
+ *tcategory = JSONBTYPE_BOOL;
+ break;
+
+ case INT2OID:
+ case INT4OID:
+ case INT8OID:
+ case FLOAT4OID:
+ case FLOAT8OID:
+ case NUMERICOID:
+ *tcategory = JSONBTYPE_NUMERIC;
+ break;
+
+ case TIMESTAMPOID:
+ *tcategory = JSONBTYPE_TIMESTAMP;
+ break;
+
+ case TIMESTAMPTZOID:
+ *tcategory = JSONBTYPE_TIMESTAMPTZ;
+ break;
+
+ case JSONBOID:
+ *tcategory = JSONBTYPE_JSONB;
+ break;
+
+ case JSONOID:
+ *tcategory = JSONBTYPE_JSON;
+ break;
+
+ default:
+ /* Check for arrays and composites */
+ if (OidIsValid(get_element_type(typoid)))
+*tcategory = JSONBTYPE_ARRAY;
+ else if (type_is_rowtype(typoid))
+*tcategory = JSONBTYPE_COMPOSITE;
+ else
+ {
+/* It's probably the general case ... */
+
Re: [HACKERS] [9.4 bug] The database server hangs with write-heavy workload on Windows
On 2014-10-13 17:56:10 +0300, Heikki Linnakangas wrote: So the gist of the problem is that LWLockRelease doesn't wake up LW_WAIT_UNTIL_FREE waiters, when releaseOK == false. It should, because a LW_WAIT_UNTIL FREE waiter is now free to run if the variable has changed in value, and it won't steal the lock from the other backend that's waiting to get the lock in exclusive mode, anyway. I'm not a big fan of that change. Right now we don't iterate the waiters if releaseOK isn't set. Which is good for the normal lwlock code because it avoids pointer indirections (of stuff likely residing on another cpu). Wouldn't it be more sensible to reset releaseOK in *UpdateVar()? I might just miss something here. I noticed another potential bug: LWLockAcquireCommon doesn't use a volatile pointer when it sets the value of the protected variable: /* If there's a variable associated with this lock, initialize it */ if (valptr) *valptr = val; /* We are done updating shared state of the lock itself. */ SpinLockRelease(lock-mutex); If the compiler or CPU decides to reorder those two, so that the variable is set after releasing the spinlock, things will break. Good catch. As Robert says that should be fine with master, but 9.4 obviously needs it. Greetings, Andres Freund -- Andres Freund 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] bad estimation together with large work_mem generates terrible slow hash joins
Kevin Grittner kgri...@ymail.com wrote: Since both Heikki and Robert spent time on this patch earlier, I'll give either of them a shot at committing it if they want; otherwise I'll do it. Done. Thanks, Tomas! -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL 9.4 mmap(2) performance regression on FreeBSD...
On Mon, Oct 13, 2014 at 05:21:32PM +0200, Andres Freund wrote: If we have it, we should improve it, or remove it. We might want to use this code for something else in the future, so it should be improved where feasible. Meh. We don't put in effort into code that doesn't matter just because it might get used elsewhere some day. By that argument we'd need to performance optimize a lot of code. And actually, using that code somewhere else is more of a counter indication than a pro argument. MAP_NOSYNC isn't a general purpose flag. The key is that this is platform-specific behavior, so if we should use a flag to use it right, we should. You are right that optimizing rarely used code with generic calls isn't a good use of time. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hide 'Execution time' in EXPLAIN (COSTS OFF)
Re: Tom Lane 2014-10-12 19766.1413129...@sss.pgh.pa.us Another possibility, which would introduce less non-orthogonality into the switch design, is to remove the connection to COSTS OFF but say that planning time is only printed when execution time is also printed (ie, only in EXPLAIN ANALYZE). This seems to me that it would not be removing much functionality, because if you just did a plain EXPLAIN then you can take the client-side runtime (psql \timing) as a close-enough estimate of planning time. I like that idea. Also, while the planning time is real even when doing a plain EXPLAIN, people who are interested in runtime behavior will be running full EXPLAIN (ANALYZE) anyway. My original suggestion to let (TIMING OFF) control it would allow for more flexibility, but as noted it isn't 100% in line with the other options, and this new idea should even be much simpler to implement or maintain. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- 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] Hide 'Execution time' in EXPLAIN (COSTS OFF)
Robert Haas robertmh...@gmail.com writes: On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane t...@sss.pgh.pa.us wrote: I have no great objection to making both COSTS OFF and TIMING OFF suppress the planning time output, if that's the consensus. I would object to taking away that behavior of COSTS OFF, because of the implications for back-patching EXPLAIN queries in regression tests. Another possibility, which would introduce less non-orthogonality into the switch design, is to remove the connection to COSTS OFF but say that planning time is only printed when execution time is also printed (ie, only in EXPLAIN ANALYZE). This seems to me that it would not be removing much functionality, because if you just did a plain EXPLAIN then you can take the client-side runtime (psql \timing) as a close-enough estimate of planning time. That'd be fine with me. Making it controlled by COSTS and/or TIMING would be OK with me, too. But let's do *something*. After sleeping on it, the second idea seems cleaner to me: it removes one wart rather than adding a second one. If there are no objections, I'll go make it so. 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] Hide 'Execution time' in EXPLAIN (COSTS OFF)
On 2014-10-13 11:46:16 -0400, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane t...@sss.pgh.pa.us wrote: I have no great objection to making both COSTS OFF and TIMING OFF suppress the planning time output, if that's the consensus. I would object to taking away that behavior of COSTS OFF, because of the implications for back-patching EXPLAIN queries in regression tests. Another possibility, which would introduce less non-orthogonality into the switch design, is to remove the connection to COSTS OFF but say that planning time is only printed when execution time is also printed (ie, only in EXPLAIN ANALYZE). This seems to me that it would not be removing much functionality, because if you just did a plain EXPLAIN then you can take the client-side runtime (psql \timing) as a close-enough estimate of planning time. That'd be fine with me. Making it controlled by COSTS and/or TIMING would be OK with me, too. But let's do *something*. After sleeping on it, the second idea seems cleaner to me: it removes one wart rather than adding a second one. If there are no objections, I'll go make it so. Well. Unless I miss something it doesn't resolve the problem that started this thread. Namely that it's currently impossible to write regression using EXPLAIN (ANALYZE, TIMING OFF. COSTS OFF). Which is worthwhile because it allows to tests some behaviour that's only visible in actually executed plans (like seing that a subtree wasn't executed). I think we should try to find a solution that solves the problem for execution/plan time problem at the same time... We could just make TIMING a tristate variable (really-off, off, on). Not very satisfying given that we have to preserve the current behaviour with OFF :(. Greetings, Andres Freund -- Andres Freund 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] [9.4 bug] The database server hangs with write-heavy workload on Windows
On 10/13/2014 06:26 PM, Andres Freund wrote:
On 2014-10-13 17:56:10 +0300, Heikki Linnakangas wrote:
So the gist of the problem is that LWLockRelease doesn't wake up
LW_WAIT_UNTIL_FREE waiters, when releaseOK == false. It should, because a
LW_WAIT_UNTIL FREE waiter is now free to run if the variable has changed in
value, and it won't steal the lock from the other backend that's waiting to
get the lock in exclusive mode, anyway.
I'm not a big fan of that change. Right now we don't iterate the waiters
if releaseOK isn't set. Which is good for the normal lwlock code because
it avoids pointer indirections (of stuff likely residing on another
cpu).
I can't get excited about that. It's pretty rare for releaseOK to be
false, and when it's true, you iterate the waiters anyway.
Wouldn't it be more sensible to reset releaseOK in *UpdateVar()? I
might just miss something here.
That's not enough. There's no LWLockUpdateVar involved in the example I
gave. And LWLockUpdateVar() already wakes up all LW_WAIT_UNTIL_FREE
waiters, regardless of releaseOK.
Hmm, we could set releaseOK in LWLockWaitForVar(), though, when it
(re-)queues the backend. That would be less invasive, for sure
(attached). I like this better.
BTW, attached is a little test program I wrote to reproduce this more
easily. It exercises the LWLock* calls directly. To run, make and
install, and do CREATE EXTENSION lwlocktest. Then launch three
backends concurrently that run select lwlocktest(1), select
lwlocktest(2) and select lwlocktest(3). It will deadlock within seconds.
- Heikki
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 5453549..cee3f08 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -482,6 +482,7 @@ static inline bool
LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
{
volatile LWLock *lock = l;
+ volatile uint64 *valp = valptr;
PGPROC *proc = MyProc;
bool retry = false;
bool result = true;
@@ -637,8 +638,8 @@ LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
}
/* If there's a variable associated with this lock, initialize it */
- if (valptr)
- *valptr = val;
+ if (valp)
+ *valp = val;
/* We are done updating shared state of the lock itself. */
SpinLockRelease(lock-mutex);
@@ -976,6 +977,12 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
lock-tail = proc;
lock-head = proc;
+ /*
+ * Set releaseOK, to make sure we get woken up as soon as the lock is
+ * released.
+ */
+ lock-releaseOK = true;
+
/* Can release the mutex now */
SpinLockRelease(lock-mutex);
lwlocktest.tar.gz
Description: application/gzip
--
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] Code bug or doc bug?
On Wed, Aug 27, 2014 at 06:39:21AM -0700, David G Johnston wrote: Is there a doc patch to make here? 1. Last sentence change suggestion: The target tablespace must be empty. 2. Based on Robert's comments it sounds like a You cannot change the default tablespace of the current database. comment should be added as well. Side note: I have no clue what the mapped relations Robert refers to are... I have created the attached doc patch for this. Should we backpatch this through 9.0, or just 9.4? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/doc/src/sgml/ref/alter_database.sgml b/doc/src/sgml/ref/alter_database.sgml new file mode 100644 index 3724c05..4af441e *** a/doc/src/sgml/ref/alter_database.sgml --- b/doc/src/sgml/ref/alter_database.sgml *** ALTER DATABASE replaceable class=PARAM *** 77,84 Only the database owner or a superuser can do this; you must also have create privilege for the new tablespace. This command physically moves any tables or indexes in the database's old !default tablespace to the new tablespace. Note that tables and indexes !in non-default tablespaces are not affected. /para para --- 77,86 Only the database owner or a superuser can do this; you must also have create privilege for the new tablespace. This command physically moves any tables or indexes in the database's old !default tablespace to the new tablespace. The new tablespace for !this database must be empty, and no one can be connected to the !database. Tables and indexes in non-default tablespaces are not !affected. /para para -- 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] Hide 'Execution time' in EXPLAIN (COSTS OFF)
Andres Freund and...@2ndquadrant.com writes: Well. Unless I miss something it doesn't resolve the problem that started this thread. Namely that it's currently impossible to write regression using EXPLAIN (ANALYZE, TIMING OFF. COSTS OFF). Which is worthwhile because it allows to tests some behaviour that's only visible in actually executed plans (like seing that a subtree wasn't executed). TBH, I don't particularly care about that. A new flag for turning summary timing off would answer the complaint with not too much non-orthogonality ... but I really don't find this use case compelling enough to justify adding a feature to EXPLAIN. 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] Missing IPv6 for pgbuildfarm.org
On 07/10/2014 09:15 AM, Andrew Dunstan wrote: On 07/02/2014 05:08 AM, Torsten Zuehlsdorff wrote: Hello, i've tried to setup a FreeBSD 10 machine as buildfarm-member. But it's an IPv6 only machine and there is no IPv6 for the homepage. Can anyone add support for IPv6 to it? I'm looking into this. This should now be working. The postgresql.org DNS maintainers might like to add this address to their entries for the server, too: 2604:da00:2:2:5054:ff:fe49:e8e 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] pgbench throttling latency limit
On 10/09/2014 10:39 PM, Fabien COELHO wrote: One thing bothers me with the log format. Here's an example: 0 81 4621 0 1412881037 912698 3005 0 82 6173 0 1412881037 914578 4304 0 83 skipped 0 1412881037 914578 5217 0 83 skipped 0 1412881037 914578 5099 0 83 4722 0 1412881037 916203 3108 0 84 4142 0 1412881037 918023 2333 0 85 2465 0 1412881037 919759 740 Note how the transaction counter is not incremented for skipped transactions. That's understandable, since we're not including skipped transactions in the number of transactions executed, but it means that the skipped transactions don't have a unique ID. That's annoying. Indeed. As transactions were not done, it does not make much sense to identify them. Otherwise it should report intended transactions and performed transactions, which would not help clarify the matter much. My idea of skipped transactions, which are not transactions as such, is just a health quality measurement for both the throttling process and the database latency, so I would really let it as it is. Hmm. I wonder if this is going to be a problem for programs that might try to load the log file into a database table. No using transaction ID as a unique key. Then again, you'll have to somehow deal with skipped anyway. Here's a new version of the patch. I'll sleep over it before committing, but I think it's fine now, except maybe for the unique ID thing. I saw a typo in a comment: lateny - latency. Otherwise it looks ok, and the documentation seems indeed clearer than before. Ok, committed after a few more typo-fixes. Greg Smith, I'd still appreciate it if you could take a look at this, to check how this will work for pgbench-tools. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Commitfest progress, or lack thereof
The August commitfest is still Open, with a few more patches left. The patches that remain have stayed in limbo for a long time. It's not realistic to expect anything to happen to them. I'm going to move the remaining patches to the next commitfest, and close the August one. I hate to do that, because the whole point of a commitfest is to get patches either rejected or committed, and not leave them hanging. But if nothing's happening, there's no point waiting. - 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] UPSERT wiki page, and SQL MERGE syntax
On Mon, Oct 13, 2014 at 7:46 AM, Robert Haas robertmh...@gmail.com wrote: Come on. You've had the syntax that way for a while, multiple people have objected to it, and it didn't get changed. When people renewed their objections, you said that they were not having a realistic conversation. I'm tired of getting critiqued because there's some fine point of one of the scores of emails you've sent on this topic that you feel I haven't adequately appreciated. We're in the fine point business. Obviously the issues with partial unique indexes *are* pretty esoteric. But worrying about these edge cases are the kind of thing that will get us an implementation of high enough quality to commit. They're esoteric until they affect you. I would like to avoid getting into a flame-war here where we spend a lot of time arguing about who should have said what in exactly what way, but I think it is fair to say that your emails have come across to me, and to a number of other people here, as digging in your heels and refusing to budge. I am not refusing to budge (in point of fact, on this question I have already budged; see my remarks below). I am saying: Hey, there is a reason why you're getting push back here. The reason isn't that I like WITHIN unique_index_name so much - obviously I don't. Who could? I don't immediately see why this would cause a problem. IIUC, the scenario we're talking about is: create table foo (a int, b int); create unique index foo1 on foo (a) where b = 1; create unique index foo2 on foo (a) where b = 2; If the user issues INSERT .. ON DUPLICATE (a) UPDATE, we'll fire before-insert triggers and then inspect the tuple to be inserted. If b is neither 1 nor 2, then we'll fail with an error saying that we can't support ON DUPLICATE without a relevant index to enforce uniqueness. (This can presumably re-use the same error message that would have popped out if the user done ON DUPLICATE (b), which is altogether un-indexed.) But if b is 1 or 2, then we'll search the matching index for a conflicting tuple; finding none, we'll insert; finding one, we'll do the update as per the user's instructions. Before row insert triggers might invalidate that conclusion at the last possible moment. So you'd have to recheck, which is just messy. As always with this stuff, the devil is in the details. If we work out the details, then I can come up with something that's acceptable to everyone. Would you be okay with this never working with partial unique indexes? That gives me something to work with. If it can't be made to work with them in a reasonable fashion, I would probably be OK with that, but so far I see no reason for such pessimism. At the very least I think that it would be a bad use of our resources to bend over backwards to make this work for partial unique indexes, which is what it would take. So I will proceed with the basic idea of naming columns, while not having that ever resolve partial unique indexes. I'm considering your points in this area as well as I can, but as far as I can tell you haven't actually set what's bad about it, just that it might be hazardous in some way that you don't appear to have specified, and that MySQL doesn't allow it. I am untroubled by the latter point; it is not our goal to confine our implementation to a subset of MySQL. I did - several times. I linked to the discussion [1]. I said There is a trade-off here. I am willing to go another way in that trade-off, but let's have a realistic conversation about it. And Kevin eventually said of not supporting partial unique indexes: That seems like the only sensible course, to me. At which point I agreed to do it that way [2]. So you've already won this argument. All it took was looking at my reasons for doing things that way from my perspective. If there has been digging of heals, you should consider that it might be for a reason, even if on balance you still disagree with my conclusion (which was clearly not really a firm conclusion in this instance anyway). That's all I mean here. I have been successful at talking you out of various approaches to this problem multiple times. This is not simple intransigence. [1] http://www.postgresql.org/message-id/CAM3SWZQpGf6_ME6D1vWqdCFadD7Nprdx2JrE=Hcf=93kxeh...@mail.gmail.com [2] http://www.postgresql.org/message-id/cam3swzstdchn6-iejbc20ogd8twmz6-um6o8gz2bofzxn9y...@mail.gmail.com -- 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] Missing IPv6 for pgbuildfarm.org
Andrew Dunstan wrote: [IPv6 support for buildfarm] This should now be working. The postgresql.org DNS maintainers might like to add this address to their entries for the server, too: 2604:da00:2:2:5054:ff:fe49:e8e This doesn't reverse-resolve to anything ... can that be fixed? -- Á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] Maximum number of WAL files in the pg_xlog directory
On Mon, Aug 25, 2014 at 07:12:33AM +0200, Guillaume Lelarge wrote:
Le 8 août 2014 09:08, Guillaume Lelarge guilla...@lelarge.info a écrit :
Hi,
As part of our monitoring work for our customers, we stumbled upon an issue
with our customers' servers who have a wal_keep_segments setting higher than
0.
We have a monitoring script that checks the number of WAL files in the
pg_xlog directory, according to the setting of three parameters
(checkpoint_completion_target, checkpoint_segments, and wal_keep_segments). We
usually add a percentage to the usual formula:
greatest(
(2 + checkpoint_completion_target) * checkpoint_segments + 1,
checkpoint_segments + wal_keep_segments + 1
)
And we have lots of alerts from the script for customers who set their
wal_keep_segments setting higher than 0.
So we started to question this sentence of the documentation:
There will always be at least one WAL segment file, and will normally not be
more than (2 + checkpoint_completion_target) * checkpoint_segments + 1 or
checkpoint_segments + wal_keep_segments + 1 files.
(http://www.postgresql.org/docs/9.3/static/wal-configuration.html)
While doing some tests, it appears it would be more something like:
wal_keep_segments + (2 + checkpoint_completion_target) * checkpoint_segments
+ 1
But after reading the source code (src/backend/access/transam/xlog.c), the
right formula seems to be:
wal_keep_segments + 2 * checkpoint_segments + 1
Here is how we went to this formula...
CreateCheckPoint(..) is responsible, among other things, for deleting and
recycling old WAL files. From src/backend/access/transam/xlog.c, master
branch,
line 8363:
/*
* Delete old log files (those no longer needed even for previous
* checkpoint or the standbys in XLOG streaming).
*/
if (_logSegNo)
{
KeepLogSeg(recptr, _logSegNo);
_logSegNo--;
RemoveOldXlogFiles(_logSegNo, recptr);
}
KeepLogSeg(...) function takes care of wal_keep_segments. From src/backend/
access/transam/xlog.c, master branch, line 8792:
/* compute limit for wal_keep_segments first */
if (wal_keep_segments 0)
{
/* avoid underflow, don't go below 1 */
if (segno = wal_keep_segments)
segno = 1;
else
segno = segno - wal_keep_segments;
}
IOW, the segment number (segno) is decremented according to the setting of
wal_keep_segments. segno is then sent back to CreateCheckPoint(...) via
_logSegNo. The RemoveOldXlogFiles() gets this segment number so that it can
remove or recycle all files before this segment number. This function gets the
number of WAL files to recycle with the XLOGfileslop constant, which is
defined
as:
/*
* XLOGfileslop is the maximum number of preallocated future XLOG segments.
* When we are done with an old XLOG segment file, we will recycle it as a
* future XLOG segment as long as there aren't already XLOGfileslop future
* segments; else we'll delete it. This could be made a separate GUC
* variable, but at present I think it's sufficient to hardwire it as
* 2*CheckPointSegments+1. Under normal conditions, a checkpoint will free
* no more than 2*CheckPointSegments log segments, and we want to recycle
all
* of them; the +1 allows boundary cases to happen without wasting a
* delete/create-segment cycle.
*/
#define XLOGfileslop (2*CheckPointSegments + 1)
(in src/backend/access/transam/xlog.c, master branch, line 100)
IOW, PostgreSQL will keep wal_keep_segments WAL files before the current WAL
file, and then there may be 2*CheckPointSegments + 1 recycled ones. Hence the
formula:
wal_keep_segments + 2 * checkpoint_segments + 1
And this is what we usually find in our customers' servers. We may find more
WAL files, depending on the write activity of the cluster, but in average, we
get this number of WAL files.
AFAICT, the documentation is wrong about the usual number of WAL files in
the
pg_xlog directory. But I may be wrong, in which case, the documentation isn't
clear enough for me, and should be fixed so that others can't misinterpret it
like I may have done.
Any comments? did I miss something, or should we fix the documentation?
I looked into this, and came up with more questions. Why is
checkpoint_completion_target involved in the total number of WAL
segments? If checkpoint_completion_target is 0.5 (the default), the
calculation is:
(2 + 0.5) * checkpoint_segments + 1
while if it is 0.9, it is:
(2 + 0.9) * checkpoint_segments + 1
Is this trying to estimate how many WAL files are going to be created
during the checkpoint? If so, wouldn't it be (1 +
checkpoint_completion_target), not 2 +. My logic is you have the old
WAL files being checkpointed (that's the 1), plus you have new WAL
files being created during the checkpoint, which would be
checkpoint_completion_target * checkpoint_segments, plus one for the
current
Re: [HACKERS] pgbench throttling latency limit
On 10/13/14, 1:54 PM, Heikki Linnakangas wrote: Greg Smith, I'd still appreciate it if you could take a look at this, to check how this will work for pgbench-tools. I'll do a QA pass on the committed version looking for issues, and update the toolchain I publish to be compatible with it along the way too. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Switch pg_basebackup to use -X stream instead of -X fetch by default?
On Fri, Aug 29, 2014 at 10:37:15PM +0200, Magnus Hagander wrote: On Fri, Aug 29, 2014 at 10:34 PM, Peter Eisentraut pete...@gmx.net wrote: On 8/27/14 2:55 AM, Magnus Hagander wrote: I think the easy way of doing that is to just create an xlog.tar file. Since we already create base.tar and possibly n*tablespace.tar, adding one more file shouldn't be a big problem, and would make such an implementation much easier. Would be trivial to do .tar.gz for it as well, just like for the others. That might be a way forward, but for someone who doesn't use tablespaces and just wants and all-on-one backup, this change would make that more cumbersome, because now you'd always have more than one file to deal with. It would in stream mode, which doesn't work at all. I do agree with Roberts suggestion that we shouldn't remove file mode right away - but we should change the default. It might be worth considering a mode that combines all those tar files into a super-tar. I'm personally not a user of the tar mode, so I don't know what a typical use would be, though. That would probably be useful, though a lot more difficult when you consider two separate processes writing into the same tarfile. But I agree that the format for single tablespace just gimme a bloody tarfile is quite incovenient today, in that you need a directory and we drop a base.tar in there. We should perhaps try to find a more convenient way for that specific usecase, since it probably represents the majority of users. Where are we on this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] On partitioning
On Fri, Aug 29, 2014 at 11:56:07AM -0400, Alvaro Herrera wrote: Prompted by a comment in the UPDATE/LIMIT thread, I saw Marko Tiikkaja reference Tom's post http://www.postgresql.org/message-id/1598.1399826...@sss.pgh.pa.us which mentions the possibility of a different partitioning implementation than what we have so far. As it turns out, I've been thinking about partitioning recently, so I thought I would share what I'm thinking so that others can poke holes. My intention is to try to implement this as soon as possible. I realize there hasn't been much progress on this thread, but I wanted to chime in to say I think our current partitioning implementation is too heavy administratively, error-prone, and performance-heavy. I support a redesign of this feature. I think the current mixture of inheritance, triggers/rules, and check constraints can be properly characterized as a Frankenstein solution, where we paste together parts until we get something that works --- our partitioning badly needs a redesign. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgres_fdw behaves oddly
Uh, where are we on this patch?
---
On Mon, Sep 8, 2014 at 09:30:32PM +0900, Etsuro Fujita wrote:
(2014/09/02 18:55), Etsuro Fujita wrote:
(2014/09/01 20:15), Etsuro Fujita wrote:
While working on [1], I've found that postgres_fdw behaves oddly:
I've revised the patch a bit further. Please find attached a patch.
Thanks,
Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***
*** 252,257 foreign_expr_walker(Node *node,
--- 252,265
if (var-varno == glob_cxt-foreignrel-relid
var-varlevelsup == 0)
{
+ /*
+ * System columns can't be sent to
remote.
+ *
+ * XXX: we should probably send ctid to
remote.
+ */
+ if (var-varattno 0)
+ return false;
+
/* Var belongs to foreign table */
collation = var-varcollid;
state = OidIsValid(collation) ?
FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
***
*** 1261,1266 deparseVar(Var *node, deparse_expr_cxt *context)
--- 1269,1279
if (node-varno == context-foreignrel-relid
node-varlevelsup == 0)
{
+ /*
+ * System columns shouldn't be examined here.
+ */
+ Assert(node-varattno = 0);
+
/* Var belongs to foreign table */
deparseColumnRef(buf, node-varno, node-varattno,
context-root);
}
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***
*** 20,25
--- 20,26
#include math.h
#include access/skey.h
+ #include access/sysattr.h
#include catalog/pg_class.h
#include foreign/fdwapi.h
#include miscadmin.h
***
*** 1945,1950 create_foreignscan_plan(PlannerInfo *root, ForeignPath
*best_path,
--- 1946,1953
RelOptInfo *rel = best_path-path.parent;
Index scan_relid = rel-relid;
RangeTblEntry *rte;
+ Bitmapset *attrs_used = NULL;
+ ListCell *lc;
int i;
/* it should be a base rel... */
***
*** 1993,2008 create_foreignscan_plan(PlannerInfo *root, ForeignPath
*best_path,
* bit of a kluge and might go away someday, so we intentionally leave
it
* out of the API presented to FDWs.
*/
scan_plan-fsSystemCol = false;
for (i = rel-min_attr; i 0; i++)
{
! if (!bms_is_empty(rel-attr_needed[i - rel-min_attr]))
{
scan_plan-fsSystemCol = true;
break;
}
}
return scan_plan;
}
--- 1996,2030
* bit of a kluge and might go away someday, so we intentionally leave
it
* out of the API presented to FDWs.
*/
+
+ /*
+ * Add all the attributes needed for joins or final output. Note: we
must
+ * look at reltargetlist, not the attr_needed data, because attr_needed
+ * isn't computed for inheritance child rels.
+ */
+ pull_varattnos((Node *) rel-reltargetlist, rel-relid, attrs_used);
+
+ /* Add all the attributes used by restriction clauses. */
+ foreach(lc, rel-baserestrictinfo)
+ {
+ RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+
+ pull_varattnos((Node *) rinfo-clause, rel-relid, attrs_used);
+ }
+
+ /* Are any system columns requested from rel? */
scan_plan-fsSystemCol = false;
for (i = rel-min_attr; i 0; i++)
{
! if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber,
attrs_used))
{
scan_plan-fsSystemCol = true;
break;
}
}
+ bms_free(attrs_used);
+
return scan_plan;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] On partitioning
Bruce Momjian wrote: On Fri, Aug 29, 2014 at 11:56:07AM -0400, Alvaro Herrera wrote: Prompted by a comment in the UPDATE/LIMIT thread, I saw Marko Tiikkaja reference Tom's post http://www.postgresql.org/message-id/1598.1399826...@sss.pgh.pa.us which mentions the possibility of a different partitioning implementation than what we have so far. As it turns out, I've been thinking about partitioning recently, so I thought I would share what I'm thinking so that others can poke holes. My intention is to try to implement this as soon as possible. I realize there hasn't been much progress on this thread, but I wanted to chime in to say I think our current partitioning implementation is too heavy administratively, error-prone, and performance-heavy. On the contrary, I think there was lots of progress; there's lots of useful feedback from the initial design proposal I posted. I am a bit sad to admit that I'm not working on it at the moment as I had originally planned, though, because other priorities slipped in and I am not able to work on this for a while. Therefore if someone else wants to work on this topic, be my guest -- otherwise I hope to get on it in a few months. I support a redesign of this feature. I think the current mixture of inheritance, triggers/rules, and check constraints can be properly characterized as a Frankenstein solution, where we paste together parts until we get something that works --- our partitioning badly needs a redesign. Agreed, and I don't think just hiding the stitches is good enough. -- Á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] On partitioning
On Mon, Oct 13, 2014 at 04:38:39PM -0300, Alvaro Herrera wrote: Bruce Momjian wrote: On Fri, Aug 29, 2014 at 11:56:07AM -0400, Alvaro Herrera wrote: Prompted by a comment in the UPDATE/LIMIT thread, I saw Marko Tiikkaja reference Tom's post http://www.postgresql.org/message-id/1598.1399826...@sss.pgh.pa.us which mentions the possibility of a different partitioning implementation than what we have so far. As it turns out, I've been thinking about partitioning recently, so I thought I would share what I'm thinking so that others can poke holes. My intention is to try to implement this as soon as possible. I realize there hasn't been much progress on this thread, but I wanted to chime in to say I think our current partitioning implementation is too heavy administratively, error-prone, and performance-heavy. On the contrary, I think there was lots of progress; there's lots of useful feedback from the initial design proposal I posted. I am a bit sad to admit that I'm not working on it at the moment as I had originally planned, though, because other priorities slipped in and I am not able to work on this for a while. Therefore if someone else wants to work on this topic, be my guest -- otherwise I hope to get on it in a few months. Oh, I just meant code progress --- I agree the discussion was fruitful. I support a redesign of this feature. I think the current mixture of inheritance, triggers/rules, and check constraints can be properly characterized as a Frankenstein solution, where we paste together parts until we get something that works --- our partitioning badly needs a redesign. Agreed, and I don't think just hiding the stitches is good enough. LOL, yeah. I do training on partitioning occasionally and the potential for mistakes is huge. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: multivariate statistics / proof of concept
Hi! On 13.10.2014 09:36, Albe Laurenz wrote: Tomas Vondra wrote: attached is a WIP patch implementing multivariate statistics. I think that is pretty useful. Oracle has an identical feature called extended statistics. That's probably an entirely different thing, but it would be very nice to have statistics to estimate the correlation between columns of different tables, to improve the estimate for the number of rows in a join. I don't have a clear idea of how that should work, but from the quick look at how join selectivity estimation is implemented, I believe two things might be possible: (a) using conditional probabilities Say we have a join ta JOIN tb ON (ta.x = tb.y) Currently, the selectivity is derived from stats on the two keys. Essentially probabilities P(x), P(y), represented by the MCV lists. But if there are additional WHERE conditions on the tables, and we have suitable multivariate stats, it's possible to use conditional probabilities. E.g. if the query actually uses ... ta JOIN tb ON (ta.x = tb.y) WHERE ta.z = 10 and we have stats on (ta.x, ta.z), we can use P(x|z=10) instead. If the two columns are correlated, this might be much different. (b) using this for multi-column conditions If the join condition involves multiple columns, e.g. ON (ta.x = tb.y AND ta.p = tb.q) and we happen to have stats on (ta.x,ta.p) and (tb.y,tb.q), we may use this to compute the cardinality (pretty much as we do today). But I haven't really worked on this so far, I suspect there are various subtle issues and I certainly don't plan to address this in the first phase of the patch. Tomas -- 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] psql \watch versus \timing
On Thu, Sep 4, 2014 at 10:50:58PM +0900, Michael Paquier wrote: On Thu, Sep 4, 2014 at 1:44 PM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Aug 28, 2014 at 8:46 PM, Fujii Masao masao.fu...@gmail.com wrote: Good catch. So I will remove start_xact code later. Attached patch removes start_xact from PSQLexec. Nothing negative to say here :) Patch simply removes the second argument of PSQLexec that was set to the same value everywhere, aka false as noticed by Heikki. Comments and code blocks related to this parameter are removed, and the code compiles, passing check-world as well (just kicked the tests in case). Uh, where are we on this? Should I commit it? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] missing tab-completion for relation options
On Thu, Sep 4, 2014 at 10:29:06PM +0900, Michael Paquier wrote: On Thu, Sep 4, 2014 at 1:53 PM, Fujii Masao masao.fu...@gmail.com wrote: Attached patch adds the missing tab-completion for the relation options like autovacuum_multixact_freeze_max_age. That's a nice catch. Multixact parameters are present since 9.3. user_catalog_table since 9.4. Uh, where we on this? Should I apply the patch? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] settings without unit
On Thu, Sep 4, 2014 at 08:50:36PM -0300, Euler Taveira wrote: Hi, I noticed that a setting in pg_settings without units have NULL and as unit values ( for integer and NULL for the other ones). Could we be consistent? It is like that since units were introduced (b517e65). No unit means unit = NULL. A proposed patch is attached. Thanks. Patch applied. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Incorrect initialization of sentPtr in walsender.c
On Fri, Sep 12, 2014 at 09:16:42PM +0900, Michael Paquier wrote: On Fri, Sep 12, 2014 at 4:55 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I haven't looked at those places closely, but it seems possible that at least some of those variables are supposed to be initialized to a value smaller than any valid WAL position, rather than just Invalid. In other words, if we defined InvalidXLogRecPtr as INT64_MAX rather than 0, we would still want those variables to be initialized to zero. As I said, I didn't check the code, but before we change those that ought to be checked. Ah, OK. I just had a look at that, and receivedUpto and lastComplaint in xlog.c need to use the lowest pointer value possible as they do a couple of comparisons with other positions. This is as well the case of sentPtr in walsender.c. However, that's not the case of writePtr and flushPtr in walreceiver.c as those positions are just used for direct comparison with LogstreamResult, so we could use InvalidXLogRecPtr in this case. What do you think of the addition of a #define for the lowest possible XLOG location pointer? I've wanted that as well a couple of times when working on clients using replication connections for for example START_REPLICATION. That would be better than hardcoding a position like '0/0', and would make the current code more solid. Patch attached in case. I like this. Can we apply it Heikki? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing IPv6 for pgbuildfarm.org
On 10/13/2014 11:28 AM, Alvaro Herrera wrote: Andrew Dunstan wrote: [IPv6 support for buildfarm] This should now be working. The postgresql.org DNS maintainers might like to add this address to their entries for the server, too: 2604:da00:2:2:5054:ff:fe49:e8e This doesn't reverse-resolve to anything ... can that be fixed? Yes. -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- 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] Obsolete comment within execTuples.c
On Sun, Sep 14, 2014 at 09:15:30PM -0700, Peter Geoghegan wrote: On Sun, Sep 14, 2014 at 9:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: More generally, though, it seems like the header comments in execTuples.c are not the best place to document global behavior ... Yeah, my thoughts exactly. I applied the attached patch to at least clean up the breakage. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c new file mode 100644 index 66515f7..7f43441 *** a/src/backend/executor/execTuples.c --- b/src/backend/executor/execTuples.c *** *** 70,83 * - ExecSeqScan() calls ExecStoreTuple() to take the result * tuple from ExecProject() and place it into the result tuple slot. * ! * - ExecutePlan() calls ExecSelect(), which passes the result slot ! * to printtup(), which uses slot_getallattrs() to extract the ! * individual Datums for printing. ! * ! * At ExecutorEnd() ! * ! * - EndPlan() calls ExecResetTupleTable() to clean up any remaining ! * tuples left over from executing the query. * * The important thing to watch in the executor code is how pointers * to the slots containing tuples are passed instead of the tuples --- 70,76 * - ExecSeqScan() calls ExecStoreTuple() to take the result * tuple from ExecProject() and place it into the result tuple slot. * ! * - ExecutePlan() calls the output function. * * The important thing to watch in the executor code is how pointers * to the slots containing tuples are passed instead of the tuples -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump refactor patch to remove global variables
Alvaro Herrera wrote:
Here's the complete patch in case anyone is wondering.
I found out that with just a little bit of extra header hacking, the
whole thing ends up cleaner -- for instance, with the attached patch,
pg_backup_archiver.h is no longer needed by pg_backup_db.h. I also
tweaked things so that no .h file includes postgres_fe.h, but instead
every .c file includes it before including anything else, as is already
customary for postgres.h in backend code.
The main changes herein are:
* some routines in pg_backup_db.c/h had an argument of type
ArchiveHandle. I made them take Archive instead, and cast internally.
This is already done for some other routines.
* also in pg_backup_db.c/h, EndDBCopyMode() had an argument of type
TocEntry, and then it only uses te-tag for an error message. If I
instead pass the te-tag I can remove the TocEntry, and there is no more
need for pg_backup_archiver.h in pg_backup_db.h.
* I moved exit_horribly() from parallel.h to pg_backup_utils.h, where
prototypes for other exit routines such as exit_nicely() are already
located. (The implementation of exit_horribly() is in parallel.c, but
the prototype looks misplaced in parallel.h.)
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 17e9574..8bfc604 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -13,8 +13,11 @@
*
*-
*/
+#include postgres_fe.h
+
#include pg_backup_archiver.h
#include pg_backup_utils.h
+#include pg_dump.h
#include ctype.h
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 2e2a447..2c568cd 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -51,10 +51,11 @@
*
*-
*/
+#include postgres_fe.h
#include compress_io.h
-#include pg_backup_utils.h
#include parallel.h
+#include pg_backup_utils.h
/*--
* Compressor API
diff --git a/src/bin/pg_dump/compress_io.h b/src/bin/pg_dump/compress_io.h
index 713c78b..5a21450 100644
--- a/src/bin/pg_dump/compress_io.h
+++ b/src/bin/pg_dump/compress_io.h
@@ -15,7 +15,6 @@
#ifndef __COMPRESS_IO__
#define __COMPRESS_IO__
-#include postgres_fe.h
#include pg_backup_archiver.h
/* Initial buffer sizes used in zlib compression. */
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index 06763ed..688e9ca 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -12,7 +12,6 @@
*
*-
*/
-
#ifndef DUMPUTILS_H
#define DUMPUTILS_H
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index ceed115..3b8d584 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -18,8 +18,8 @@
#include postgres_fe.h
-#include pg_backup_utils.h
#include parallel.h
+#include pg_backup_utils.h
#ifndef WIN32
#include sys/types.h
diff --git a/src/bin/pg_dump/parallel.h b/src/bin/pg_dump/parallel.h
index 744c76b..dd3546f 100644
--- a/src/bin/pg_dump/parallel.h
+++ b/src/bin/pg_dump/parallel.h
@@ -86,8 +86,4 @@ extern void ParallelBackupEnd(ArchiveHandle *AH, ParallelState *pstate);
extern void checkAborting(ArchiveHandle *AH);
-extern void
-exit_horribly(const char *modulename, const char *fmt,...)
-__attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3), noreturn));
-
#endif /* PG_DUMP_PARALLEL_H */
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 37fdd8c..c2ebcd4 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -23,10 +23,7 @@
#ifndef PG_BACKUP_H
#define PG_BACKUP_H
-#include postgres_fe.h
-
#include dumputils.h
-
#include libpq-fe.h
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 71ac6e5..95cb6e8 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -19,10 +19,12 @@
*
*-
*/
+#include postgres_fe.h
+#include parallel.h
+#include pg_backup_archiver.h
#include pg_backup_db.h
#include pg_backup_utils.h
-#include parallel.h
#include ctype.h
#include fcntl.h
@@ -424,7 +426,7 @@ RestoreArchive(Archive *AHX)
if (ropt-single_txn)
{
if (AH-connection)
- StartTransaction(AH);
+ StartTransaction(AHX);
else
ahprintf(AH, BEGIN;\n\n);
}
@@ -642,7 +644,7 @@ RestoreArchive(Archive *AHX)
if (ropt-single_txn)
{
if (AH-connection)
- CommitTransaction(AH);
+ CommitTransaction(AHX);
else
ahprintf(AH, COMMIT;\n\n);
}
@@ -823,7 +825,7 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te,
* Parallel restore is always talking directly to a
* server, so no need
[HACKERS] Possible micro-optimization in CacheInvalidateHeapTuple
CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing the system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there any reason not to switch the two? /* Do nothing during bootstrap */ if (IsBootstrapProcessingMode()) return; /* * We only need to worry about invalidation for tuples that are in system * relations; user-relation tuples are never in catcaches and can't affect * the relcache either. */ if (!IsSystemRelation(relation)) return; -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] missing tab-completion for relation options
On Tue, Oct 14, 2014 at 4:51 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Sep 4, 2014 at 10:29:06PM +0900, Michael Paquier wrote: On Thu, Sep 4, 2014 at 1:53 PM, Fujii Masao masao.fu...@gmail.com wrote: Attached patch adds the missing tab-completion for the relation options like autovacuum_multixact_freeze_max_age. That's a nice catch. Multixact parameters are present since 9.3. user_catalog_table since 9.4. Uh, where we on this? Should I apply the patch? Committed as of d85e7fa. -- 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] psql \watch versus \timing
On Tue, Oct 14, 2014 at 4:49 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Sep 4, 2014 at 10:50:58PM +0900, Michael Paquier wrote: On Thu, Sep 4, 2014 at 1:44 PM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Aug 28, 2014 at 8:46 PM, Fujii Masao masao.fu...@gmail.com wrote: Good catch. So I will remove start_xact code later. Attached patch removes start_xact from PSQLexec. Nothing negative to say here :) Patch simply removes the second argument of PSQLexec that was set to the same value everywhere, aka false as noticed by Heikki. Comments and code blocks related to this parameter are removed, and the code compiles, passing check-world as well (just kicked the tests in case). Uh, where are we on this? Should I commit it? The patch cleaning up the dead code of psql could be clearly applied now. For the feature itself not sure, it may be better to let Fujii-san manage it. -- 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] interval typmodout is broken
On Thu, Sep 25, 2014 at 12:06:56AM -0400, Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Tom Lane wrote:
You sure about that? The grammar for INTERVAL is weird.
Well, I tested what is taken on input, and yes I agree the grammar is
weird (but not more weird than timestamp/timestamptz, mind). The input
function only accepts the precision just after the INTERVAL keyword, not
after the fieldstr:
alvherre=# create table str (a interval(2) hour to minute);
CREATE TABLE
alvherre=# create table str2 (a interval hour to minute(2));
ERROR: syntax error at or near (
L�NEA 1: create table str2 (a interval hour to minute(2));
^
No, that's not about where it is, it's about what the field is: only
second can have a precision. Our grammar is actually allowing stuff
here that it shouldn't. According to the SQL spec, you could write
interval hour(2) to minute
but this involves a leading field precision, which we do not support
and should definitely not be conflating with trailing-field precision.
Or you could write
interval hour to second(2)
which is valid and we support it. You can *not* write
interval hour to minute(2)
either per spec or per our implementation; and
interval(2) hour to minute
is 100% invalid per spec, even though our grammar goes out of its
way to accept it.
In short, the typmodout function is doing what it ought to. It's the
grammar that's broken. It looks to me like Tom Lockhart coded the
grammar to accept a bunch of cases that he never got round to actually
implementing reasonably. In particular, per SQL spec these are
completely different animals:
interval hour(2) to second
interval hour to second(2)
but our grammar transforms them into the same thing.
We ought to fix that...
I did not find any cases where we support 'INTERVAL HOUR(2) to SECOND'.
I think the basic problem is that the original author had the idea of
doing:
SELECT INTERVAL (2) '100. seconds';
interval
--
00:01:41
and using (2) in that location as a short-hand when the interval
precision units were not specified, which seems logical. However, they
allowed it even when the units were specified:
SELECT INTERVAL (2) '100. seconds' HOUR to SECOND;
interval
--
00:01:41
and in cases where the precision made no sense:
SELECT INTERVAL (2) '100. seconds' HOUR to MINUTE;
interval
--
00:01:00
I have created the attached patch which only allows parentheses in the
first case.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
new file mode 100644
index c98c27a..0de9584
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*** zone_value:
*** 1552,1578
t-typmods = $3;
$$ = makeStringConstCast($2, @2, t);
}
! | ConstInterval '(' Iconst ')' Sconst opt_interval
{
TypeName *t = $1;
! if ($6 != NIL)
! {
! A_Const *n = (A_Const *) linitial($6);
! if ((n-val.val.ival ~(INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE))) != 0)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg(time zone interval must be HOUR or HOUR TO MINUTE),
! parser_errposition(@6)));
! if (list_length($6) != 1)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg(interval precision specified twice),
! parser_errposition(@1)));
! t-typmods = lappend($6, makeIntConst($3, @3));
! }
! else
! t-typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
$$ = makeStringConstCast($5, @5, t);
}
| NumericOnly { $$ = makeAConst($1, @1); }
--- 1552,1562
t-typmods = $3;
$$ = makeStringConstCast($2, @2, t);
}
! | ConstInterval '(' Iconst ')' Sconst
{
TypeName *t = $1;
! t-typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
$$ = makeStringConstCast($5, @5, t);
}
| NumericOnly { $$ = makeAConst($1, @1); }
*** SimpleTypename:
*** 10582,10602
$$ = $1;
$$-typmods = $2;
}
! | ConstInterval '(' Iconst ')' opt_interval
{
$$ = $1;
! if ($5 != NIL)
! {
! if (list_length($5) != 1)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg(interval precision specified twice),
! parser_errposition(@1)));
! $$-typmods = lappend($5, makeIntConst($3, @3));
! }
! else
! $$-typmods =
Re: [HACKERS] postgres_fdw behaves oddly
On Mon, Oct 13, 2014 at 3:30 PM, Bruce Momjian br...@momjian.us wrote: Uh, where are we on this patch? Probably it should be added to the upcoming CF. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_get_indexdef() doesn't quote string reloptions
On Mon, Oct 13, 2014 at 11:21 AM, Eric Ridge e_ri...@tcdi.com wrote: PROPRIETARY AND COMPANY CONFIDENTIAL COMMUNICATIONS The information contained in this communication is intended only for the use of the addressee. Any other use is strictly prohibited. Please notify the sender if you have received this message in error. This communication is protected by applicable legal privileges and is company confidential. If this communication is in fact intended to be protected by some legal privilege, or to remain company confidential, you have definitely sent it to the wrong place. If it isn't, I think it shouldn't say that 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
[HACKERS] Typo in bgworker.sgml
I found a minor typo in bgworker.sgml. Patch attached. It should be applied to 9.4 too. -- Shigeru HANADA fix_typo_in_bgworker.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_get_indexdef() doesn't quote string reloptions
On Tue, Oct 14, 2014 at 12:21 AM, Eric Ridge e_ri...@tcdi.com wrote:
pg_get_indexdef() and pg_dump don't quote the reloption values, making a
restore (or cut-n-paste of the pg_get_indexdef() output) impossible if the
reloption value contains non-alphanumerics.
For example, the statement:
# CREATE INDEX idxfoo ON table USING myindex (col) WITH (option =
'some complex string');
cannot be restored as it gets rewritten as:
CREATE INDEX idxfoo ON table USING myindex (col) WITH (option = some
complex string);
(note the lack of quotes around the option value)
Looks like (at least) ruleutils.c:flatten_reloptions() needs to be smarter.
The limitation is not directly related to ruleutils.c, but to the way
reloptions are stored for a relation: no quotes are being used
because, well, they are not necessary. All the custom parameters that
can be used by tables or indexes are either on/off switches or
integers. For example:
=# CREATE TABLE test_trgm (t text);
CREATE TABLE
=# CREATE INDEX trgm_idx_gin ON test_trgm USING gin (t gin_trgm_ops)
WITH (fastupdate = off);
CREATE INDEX
=# CREATE INDEX trgm_idx_gist ON test_trgm USING gist (t
gist_trgm_ops) WITH (buffering = on);
CREATE INDEX
=# CREATE TABLE aa (a int) WITH (fillfactor = 40);
CREATE TABLE
=# SELECT relname, reloptions FROM pg_class where relname in
('trgm_idx_gin','trgm_idx_gist','aa');
relname|reloptions
---+--
trgm_idx_gin | {fastupdate=off}
trgm_idx_gist | {buffering=on}
aa| {fillfactor=40}
(3 rows)
Now, this problem has been discussed a couple of weeks ago when
arguing about adding unit support for storage parameters. Here is
where the feature has been discussed:
http://www.postgresql.org/message-id/flat/CAHGQGwEanQ_e8WLHL25=bm_8z5zkyzw0k0yir+kdmv2hgne...@mail.gmail.com#CAHGQGwEanQ_e8WLHL25=bm_8z5zkyzw0k0yir+kdmv2hgne...@mail.gmail.com
And the thread where the limitation has been actually found:
http://www.postgresql.org/message-id/cab7npqsevwnhk-ta-gjbdgea-1zlt8wfywsp_63ut2ia8w9...@mail.gmail.com
Your need is an argument to make reloptions smarter with quotes. Not
sure that's on the top of the TODO list of people here though.
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible micro-optimization in CacheInvalidateHeapTuple
Jim Nasby jim.na...@bluetreble.com writes: CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing the system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there any reason not to switch the two? /* Do nothing during bootstrap */ if (IsBootstrapProcessingMode()) return; /* * We only need to worry about invalidation for tuples that are in system * relations; user-relation tuples are never in catcaches and can't affect * the relcache either. */ if (!IsSystemRelation(relation)) return; You're assuming that IsSystemRelation() is safe to apply during bootstrap mode. Even if it is, I don't see the point of messing with this. IsBootstrapProcessingMode() is a macro expanding to one comparison instruction. 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
