pgsql: Minor copy-editing for 10.2 release notes.
Minor copy-editing for 10.2 release notes. Second pass after taking a break ... Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/794eb3a8f00519fac561831dee35f4ee557bd08b Modified Files -- doc/src/sgml/release-10.sgml | 41 - 1 file changed, 20 insertions(+), 21 deletions(-)
pgsql: doc: Fix index link
doc: Fix index link The index entry was pointing to a slightly wrong location. Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/dcc1e61cb23606bf3e90079a0529821939c0e9c6 Modified Files -- doc/src/sgml/logicaldecoding.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: doc: Fix index link
doc: Fix index link The index entry was pointing to a slightly wrong location. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bc38bdba04d75f7c39d57f3eba9c01958d8d2f7c Modified Files -- doc/src/sgml/logicaldecoding.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure
Thomas Munro writes: > On Sat, Feb 3, 2018 at 12:32 PM, Tom Lane wrote: >> Fix another instance of unsafe coding for shm_toc_lookup failure. > my mistake was actually to put noError = true there when noError = > false was called for. Ah. > However, it's not surprising that you drew the > opposite conclusion (ie that it might in fact not be in the TOC), > since the shm space is really only necessary for EXPLAIN ANALYZE. > Perhaps I should make it skip setting up this shm stuff if > !node->ss.ps.instrument, just like the equivalent Sort node code. I > will look into that on Monday. OK. Please send in a patch to either do that or switch this call to use noError = false. Or possibly both: shouldn't there be some other signal path that tells the worker whether instrumentation is needed? I'll leave it alone pending your investigation. regards, tom lane
Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure
On Sat, Feb 3, 2018 at 12:32 PM, Tom Lane wrote: > Fix another instance of unsafe coding for shm_toc_lookup failure. > > One or another author of commit 5bcf389ec seems to have thought that > computing an offset from a NULL pointer would yield another NULL pointer. > There may possibly be architectures where that works, but common machines > don't work like that. Per a quick code review of places calling > shm_toc_lookup and not using noError = false. Hmm... That was me. FWIW I certainly didn't think that about pointer arithmetic... I think I must have got confused about the sense of that flag. ExecHashInitializeWorker() should always find a TOC entry using plan_node_id, because ExecHashInitializeDSM() always inserts one, so my mistake was actually to put noError = true there when noError = false was called for. However, it's not surprising that you drew the opposite conclusion (ie that it might in fact not be in the TOC), since the shm space is really only necessary for EXPLAIN ANALYZE. Perhaps I should make it skip setting up this shm stuff if !node->ss.ps.instrument, just like the equivalent Sort node code. I will look into that on Monday. -- Thomas Munro http://www.enterprisedb.com
pgsql: Be more wary about shm_toc_lookup failure.
Be more wary about shm_toc_lookup failure. Commit 445dbd82a basically missed the point of commit d46633506, which was that we shouldn't allow shm_toc_lookup() failure to lead to a core dump or assertion crash, because the odds of such a failure should never be considered negligible. It's correct that we can't expect the PARALLEL_KEY_ERROR_QUEUE TOC entry to be there if we have no workers. But if we have no workers, we're not going to do anything in this function with the lookup result anyway, so let's just skip it. That lets the code use the easy-to-prove-safe noError=false case, rather than anything requiring effort to review. Back-patch to v10, like the previous commit. Discussion: https://postgr.es/m/3647.1517601...@sss.pgh.pa.us Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/34653bc9833d9c95ed1c52bed1ff01c6193de17b Modified Files -- src/backend/access/transam/parallel.c | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-)
pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure
Fix another instance of unsafe coding for shm_toc_lookup failure. One or another author of commit 5bcf389ec seems to have thought that computing an offset from a NULL pointer would yield another NULL pointer. There may possibly be architectures where that works, but common machines don't work like that. Per a quick code review of places calling shm_toc_lookup and not using noError = false. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d59ff4ab3111f20054425d82dab1393101dcfe8e Modified Files -- src/backend/executor/nodeHash.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-)
pgsql: Be more wary about shm_toc_lookup failure.
Be more wary about shm_toc_lookup failure. Commit 445dbd82a basically missed the point of commit d46633506, which was that we shouldn't allow shm_toc_lookup() failure to lead to a core dump or assertion crash, because the odds of such a failure should never be considered negligible. It's correct that we can't expect the PARALLEL_KEY_ERROR_QUEUE TOC entry to be there if we have no workers. But if we have no workers, we're not going to do anything in this function with the lookup result anyway, so let's just skip it. That lets the code use the easy-to-prove-safe noError=false case, rather than anything requiring effort to review. Back-patch to v10, like the previous commit. Discussion: https://postgr.es/m/3647.1517601...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/957ff087c822c95f63df956e1a91c15614ecb2b4 Modified Files -- src/backend/access/transam/parallel.c | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-)
pgsql: First-draft release notes for 10.2.
First-draft release notes for 10.2. As usual, the release notes for other branches will be made by cutting these down, but put them up for community review first. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bf641d3376018c40860f26167a60febff5bc1f51 Modified Files -- doc/src/sgml/release-10.sgml | 1164 ++ 1 file changed, 1164 insertions(+)
pgsql: Fix application of identity values in some cases
Fix application of identity values in some cases Investigation of 2d2d06b7e27e3177d5bef0061801c75946871db3 revealed that identity values were not applied in some further cases, including logical replication subscribers, VALUES RTEs, and ALTER TABLE ... ADD COLUMN. To fix all that, apply the identity column expression in build_column_default() instead of repeating the same logic at each call site. For ALTER TABLE ... ADD COLUMN ... IDENTITY, the previous coding completely ignored that existing rows for the new column should have values filled in from the identity sequence. The coding using build_column_default() fails for this because the sequence ownership isn't registered until after ALTER TABLE, and we can't do it before because we don't have the column in the catalog yet. So we specially remember in ColumnDef the sequence name that we decided on and build a custom NextValueExpr using that. Reviewed-by: Michael Paquier Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/533c5d8bddf0feb1785b3da17c0d17feeaac76d8 Modified Files -- src/backend/commands/copy.c| 16 ++-- src/backend/commands/tablecmds.c | 16 +++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/parser/parse_utilcmd.c | 8 src/backend/rewrite/rewriteHandler.c | 22 +++--- src/include/nodes/parsenodes.h | 2 ++ src/test/regress/expected/identity.out | 28 src/test/regress/sql/identity.sql | 19 +++ src/test/subscription/t/008_diff_schema.pl | 18 ++ 11 files changed, 102 insertions(+), 30 deletions(-)
pgsql: Fix application of identity values in some cases
Fix application of identity values in some cases Investigation of 2d2d06b7e27e3177d5bef0061801c75946871db3 revealed that identity values were not applied in some further cases, including logical replication subscribers, VALUES RTEs, and ALTER TABLE ... ADD COLUMN. To fix all that, apply the identity column expression in build_column_default() instead of repeating the same logic at each call site. For ALTER TABLE ... ADD COLUMN ... IDENTITY, the previous coding completely ignored that existing rows for the new column should have values filled in from the identity sequence. The coding using build_column_default() fails for this because the sequence ownership isn't registered until after ALTER TABLE, and we can't do it before because we don't have the column in the catalog yet. So we specially remember in ColumnDef the sequence name that we decided on and build a custom NextValueExpr using that. Reviewed-by: Michael Paquier Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1597948c962a1407c01fc492c44917c097efa92e Modified Files -- src/backend/commands/copy.c| 16 ++-- src/backend/commands/tablecmds.c | 16 +++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/parser/parse_utilcmd.c | 8 src/backend/rewrite/rewriteHandler.c | 22 +++--- src/include/nodes/parsenodes.h | 2 ++ src/test/regress/expected/identity.out | 28 src/test/regress/sql/identity.sql | 19 +++ src/test/subscription/t/008_diff_schema.pl | 18 ++ 11 files changed, 102 insertions(+), 30 deletions(-)
pgsql: Support parallel btree index builds.
Support parallel btree index builds. To make this work, tuplesort.c and logtape.c must also support parallelism, so this patch adds that infrastructure and then applies it to the particular case of parallel btree index builds. Testing to date shows that this can often be 2-3x faster than a serial index build. The model for deciding how many workers to use is fairly primitive at present, but it's better than not having the feature. We can refine it as we get more experience. Peter Geoghegan with some help from Rushabh Lathia. While Heikki Linnakangas is not an author of this patch, he wrote other patches without which this feature would not have been possible, and therefore the release notes should possibly credit him as an author of this feature. Reviewed by Claudio Freire, Heikki Linnakangas, Thomas Munro, Tels, Amit Kapila, me. Discussion: http://postgr.es/m/CAM3SWZQKM=Pzc=cahzrixkjp2eo5q0jg1sofqqexfq647ji...@mail.gmail.com Discussion: http://postgr.es/m/CAH2-Wz=axwqdovvgu7dq856s4r6sjaj6dbn7vmtigkb33n5...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9da0cc35284bdbe8d442d732963303ff0e0a40bc Modified Files -- contrib/bloom/blinsert.c | 3 +- doc/src/sgml/config.sgml | 44 +- doc/src/sgml/monitoring.sgml | 12 +- doc/src/sgml/ref/create_index.sgml| 58 ++ doc/src/sgml/ref/create_table.sgml| 4 +- src/backend/access/brin/brin.c| 4 +- src/backend/access/gin/gininsert.c| 2 +- src/backend/access/gist/gistbuild.c | 2 +- src/backend/access/hash/hash.c| 2 +- src/backend/access/hash/hashsort.c| 1 + src/backend/access/heap/heapam.c | 28 +- src/backend/access/nbtree/nbtree.c| 134 +--- src/backend/access/nbtree/nbtsort.c | 878 +- src/backend/access/spgist/spginsert.c | 3 +- src/backend/access/transam/parallel.c | 12 +- src/backend/bootstrap/bootstrap.c | 2 +- src/backend/catalog/heap.c| 2 +- src/backend/catalog/index.c | 123 +++- src/backend/catalog/toasting.c| 1 + src/backend/commands/cluster.c| 3 +- src/backend/commands/indexcmds.c | 7 +- src/backend/executor/execParallel.c | 2 +- src/backend/executor/nodeAgg.c| 6 +- src/backend/executor/nodeSort.c | 2 +- src/backend/optimizer/path/allpaths.c | 18 +- src/backend/optimizer/path/costsize.c | 4 +- src/backend/optimizer/plan/planner.c | 136 src/backend/postmaster/pgstat.c | 3 + src/backend/storage/file/buffile.c| 61 +- src/backend/storage/file/fd.c | 10 + src/backend/utils/adt/orderedsetaggs.c| 2 + src/backend/utils/init/globals.c | 1 + src/backend/utils/misc/guc.c | 10 + src/backend/utils/misc/postgresql.conf.sample | 3 +- src/backend/utils/probes.d| 2 +- src/backend/utils/sort/logtape.c | 199 +- src/backend/utils/sort/tuplesort.c| 595 ++--- src/include/access/nbtree.h | 14 +- src/include/access/parallel.h | 4 +- src/include/access/relscan.h | 1 + src/include/catalog/index.h | 9 +- src/include/miscadmin.h | 1 + src/include/nodes/execnodes.h | 6 +- src/include/optimizer/paths.h | 2 +- src/include/optimizer/planner.h | 1 + src/include/pgstat.h | 1 + src/include/storage/buffile.h | 2 + src/include/storage/fd.h | 1 + src/include/utils/logtape.h | 39 +- src/include/utils/tuplesort.h | 132 +++- src/tools/pgindent/typedefs.list | 6 + 51 files changed, 2237 insertions(+), 361 deletions(-)
Re: pgsql: Remove byte-masking macros for Datum conversion macros
Peter Eisentraut writes: > Remove byte-masking macros for Datum conversion macros Looking at the code now, I think there's at least one bad outcome of this change: the behavior of CharGetDatum() is now ill-defined, because we'll (probably) get different results on signed-char and unsigned-char compilers. I think we'd be well advised to make that macro be #define CharGetDatum(X) ((Datum) ((unsigned char) (X))) regards, tom lane
pgsql: Refactor code for partition bound searching
Refactor code for partition bound searching Remove partition_bound_cmp() and partition_bound_bsearch(), whose void * argument could be, depending on the situation, of any of three different types: PartitionBoundSpec *, PartitionRangeBound *, Datum *. Instead, introduce separate bound-searching functions for each situation: partition_list_bsearch, partition_range_bsearch, partition_range_datum_bsearch, and partition_hash_bsearch. This requires duplicating the code for binary search, but it makes the code much more type safe, involves fewer branches at runtime, and at least in my opinion, is much easier to understand. Along the way, add an option to partition_range_datum_bsearch allowing the number of keys to be specified, so that we can search for partitions based on a prefix of the full list of partition keys. This is important for pending work to improve partition pruning. Amit Langote, per a suggestion from me. Discussion: http://postgr.es/m/CA+TgmoaVLDLc8=yesrwd32gphodu_elmxyks77gveiyp+je...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9aef173163ae68c6b241e4c9bbb375c6baa71c60 Modified Files -- src/backend/catalog/partition.c | 265 ++-- 1 file changed, 170 insertions(+), 95 deletions(-)
pgsql: Add new function WaitForParallelWorkersToAttach.
Add new function WaitForParallelWorkersToAttach. Once this function has been called, we know that all workers have started and attached to their error queues -- so if any of them subsequently exit uncleanly, we'll be sure to throw an ERROR promptly. Otherwise, users of the ParallelContext machinery must be careful not to wait forever for a worker that has failed to start. Parallel query manages to work without needing this for reasons explained in new comments added by this patch, but it's a useful primitive for other parallel operations, such as the pending patch to make creating a btree index run in parallel. Amit Kapila, revised by me. Additional review by Peter Geoghegan. Discussion: http://postgr.es/m/CAA4eK1+e2MzyouF5bg=OtyhDSX+=Ao=3htN=t-r_6s3gctk...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9222c0d9ed9794d54fc3f5101498829eaec9e799 Modified Files -- src/backend/access/transam/parallel.c | 152 +++-- src/backend/executor/nodeGather.c | 9 +- src/backend/executor/nodeGatherMerge.c | 9 +- src/include/access/parallel.h | 4 +- 4 files changed, 163 insertions(+), 11 deletions(-)
pgsql: Improve ALTER TABLE synopsis
Improve ALTER TABLE synopsis Add into the ALTER TABLE synopsis the definition of partition_bound_spec, column_constraint, index_parameters and exclude_element. Initial patch by Lætitia Avrot, with further improvements by Amit Langote and Thomas Munro. Discussion: https://postgr.es/m/flat/27ec4df3-d1ab-3411-f87f-647f944897e1%40lab.ntt.co.jp Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/a2a22057617dc84b500f85938947c125183f1289 Modified Files -- doc/src/sgml/ref/alter_table.sgml | 31 +++ 1 file changed, 31 insertions(+)
