Re: [HACKERS] Building PL/Perl with ActiveState Perl 5.22 and MSVC

[Noah Misch]

On Sat, Mar 26, 2016 at 03:43:21PM +0300, Victor Wagner wrote:
> It turns out that while ActiveState seems to drop support of embedding
> their perl into msvc-compiled appications, there are just few minor
> issues which prevent PL-perl to compile.
> 
> 1. ActiveState Perl doesn't ship MSVC-build import library perl522.lib
> for their perl522.dll. Instead they ship MINGW-build library
> libperl522.a.
> 
> Visual Studio 2012 is able to link against this library. It is only
> matter of modifing search expresions in Mkvcbuild.pm to be able to find
> this import library. Not sure if it stands true for all eariler
> versions of Visual Studio, supported by Postgresql.
> 
> 2. There is macro PERL_STATIC_INLINME in perl's lib/CORE/config.h file,
> which produces compilation errors.

> #define PERL_STATIC_INLINE static __inline__  /**/

I've seen the same problems, and I converted your description into the
attached patch.  With ActivePerl-5.24.1.2402-MSWin32-x64-401627.exe binaries,
"vcregress plcheck" passes.  I plan to back-patch this.  If some site has
worked around this with "copy libperl522.a perl522.lib", that site's build
will fail due to having two matching libraries.  The build failure and fix
will be clear enough, so that seems acceptable.

> 3. Fixing two issues above was enough to make build complete for 64-bit
> windows target. With 32-bit build I'v got linking errors 
> LINK2026: module unsafe for safeseh image
> 
> for all modules which are linked with perl DLL. I suspect that it is
> not  a problem with DLL itself, it is rather related to way MINGW32
> generates its import libraries. To fix this problem XML element
> false
> should be added inside  element of the two vcxproj files which
> link with perl - plperl.vcxproj and hstore_plperl.vcxproj.

Official 10.1 x86 binaries[1] contain a SAFESEH build of plperl.dll, linked
with a perl524.dll.  I wonder how.  Do they use a perl524.dll from a source
other than ActivePerl?  Before adopting your proposal, I'd like to understand
why the official binaries haven't needed it.  Also, I'd instead use
"", which omits /SAFESEH entirely instead of
passing /SAFESEH:NO.  That way, only the binaries linked to Perl (or other
old-fashioned DLLs) lose their safe exception handler table.

Even if I do this, "vcregress plcheck" fails with "loadable library and perl
binaries are mismatched (got handshake key 0B080080, needed 0AF00080)".
That's building with ActivePerl-5.22.4.2205-MSWin32-x86-64int-403863.exe and
VS2015.  When I have more information, I'll send it to the thread[2] about
that failure mode.

Thanks,
nm

[1] 
https://get.enterprisedb.com/postgresql/postgresql-10.1-1-windows-binaries.zip
[2] 
https://postgr.es/m/flat/CAE9k0P%3DhMFew%3DVxNGTeOJJr32%2BUDy-o2qWXrThHg524EBqvnZQ%40mail.gmail.com
diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h
index c4e06d0..aac95f8 100644
--- a/src/pl/plperl/plperl.h
+++ b/src/pl/plperl/plperl.h
@@ -42,6 +42,14 @@
 #undef vsnprintf
 #endif
 
+/*
+ * ActivePerl 5.18 and later are MinGW-built, and their headers use GCC's
+ * __inline__.  Translate to something MSVC recognizes.
+ */
+#ifdef _MSC_VER
+#define __inline__ inline
+#endif
+
 
 /*
  * Get the basic Perl API.  We use PERL_NO_GET_CONTEXT mode so that our code
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 686c736..4c2e12e 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -615,9 +615,10 @@ sub mkvcbuild
}
}
$plperl->AddReference($postgres);
-   my $perl_path = $solution->{options}->{perl} . 
'\lib\CORE\perl*.lib';
+   my $perl_path = $solution->{options}->{perl} . 
'\lib\CORE\*perl*';
+   # ActivePerl 5.16 provided perl516.lib; 5.18 provided 
libperl518.a
my @perl_libs =
- grep { /perl\d+.lib$/ } glob($perl_path);
+ grep { /perl\d+\.lib$|libperl\d+\.a$/ } glob($perl_path);
if (@perl_libs == 1)
{
$plperl->AddLibrary($perl_libs[0]);

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

Re: [HACKERS] Parallel Append implementation

[Amit Khandekar]

Thanks a lot Robert for the patch. I will have a look. Quickly tried
to test some aggregate queries with a partitioned pgbench_accounts
table, and it is crashing. Will get back with the fix, and any other
review comments.

Thanks
-Amit Khandekar

On 9 November 2017 at 23:44, Robert Haas  wrote:
> On Sat, Oct 28, 2017 at 5:50 AM, Robert Haas  wrote:
>> No, because the Append node is *NOT* getting copied into shared
>> memory.  I have pushed a comment update to the existing functions; you
>> can use the same comment for this patch.
>
> I spent the last several days working on this patch, which had a
> number of problems both cosmetic and functional.  I think the attached
> is in better shape now, but it could certainly use some more review
> and testing since I only just finished modifying it, and I modified it
> pretty heavily.  Changes:
>
> - I fixed the "morerows" entries in the documentation.  If you had
> built the documentation the way you had it and loaded up in a web
> browser, you would have seen that the way you had it was not correct.
>
> - I moved T_AppendState to a different position in the switch inside
> ExecParallelReInitializeDSM, so as to keep that switch in the same
> order as all of the other switch statements in that file.
>
> - I rewrote the comment for pa_finished.  It previously began with
> "workers currently executing the subplan", which is not an accurate
> description. I suspect this was a holdover from a previous version of
> the patch in which this was an array of integers rather than an array
> of type bool.  I also fixed the comment in ExecAppendEstimate and
> added, removed, or rewrote various other comments as well.
>
> - I renamed PA_INVALID_PLAN to INVALID_SUBPLAN_INDEX, which I think is
> more clear and allows for the possibility that this sentinel value
> might someday be used for non-parallel-aware Append plans.
>
> - I largely rewrote the code for picking the next subplan.  A
> superficial problem with the way that you had it is that you had
> renamed exec_append_initialize_next to exec_append_seq_next but not
> updated the header comment to match.  Also, the logic was spread out
> all over the file.  There are three cases: not parallel aware, leader,
> worker.  You had the code for the first case at the top of the file
> and the other two cases at the bottom of the file and used multiple
> "if" statements to pick the right one in each case.  I replaced all
> that with a function pointer stored in the AppendState, moved the code
> so it's all together, and rewrote it in a way that I find easier to
> understand.  I also changed the naming convention.
>
> - I renamed pappend_len to pstate_len and ParallelAppendDescData to
> ParallelAppendState.  I think the use of the word "descriptor" is a
> carryover from the concept of a scan descriptor.  There's nothing
> really wrong with inventing the concept of an "append descriptor", but
> it seems more clear to just refer to shared state.
>
> - I fixed ExecAppendReInitializeDSM not to reset node->as_whichplan.
> Per commit 41b0dd987d44089dc48e9c70024277e253b396b7, that's wrong;
> instead, local state should be reset in ExecReScanAppend.  I installed
> what I believe to be the correct logic in that function instead.
>
> - I fixed list_qsort() so that it copies the type of the old list into
> the new list.  Otherwise, sorting a list of type T_IntList or
> T_OidList would turn it into just plain T_List, which is wrong.
>
> - I removed get_append_num_workers and integrated the logic into the
> callers.  This function was coded quite strangely: it assigned the
> return value of fls() to a double and then eventually rounded the
> result back to an integer.  But fls() returns an integer, so this
> doesn't make much sense.  On a related note, I made it use fls(# of
> subpaths) instead of fls(# of subpaths)+1.  Adding 1 doesn't make
> sense to me here because it leads to a decision to use 2 workers for a
> single, non-partial subpath.  I suspect both of these mistakes stem
> from thinking that fls() returns the base-2 logarithm, but in fact it
> doesn't, quite: log2(1) = 0.0 but fls(1) = 1.
>
> - In the process of making the changes described in the previous
> point, I added a couple of assertions, one of which promptly failed.
> It turns out the reason is that your patch didn't update
> accumulate_append_subpaths(), which can result in flattening
> non-partial paths from a Parallel Append into a parent Append's list
> of partial paths, which is bad.  The easiest way to fix that would be
> to just teach accumulate_append_subpaths() not to flatten a Parallel
> Append into a parent Append or MergeAppend node, but it seemed to me
> that there was a fair amount of duplication between
> accumulate_partialappend_subpath() and accumulate_append_subpaths, so
> what I did instead is folded all of the necessarily logic directly
> into accumulate_append_subpaths().  This approach also avoids some
> assumptions that your code made, such as the assum

Re: [HACKERS] Variable substitution in psql backtick expansion

[Fabien COELHO]

Hello Tom & Michaël,


I think this patch should be rejected.

+1 for rejection [...]


The noes have it!

Note that the motivation was really symmetric completion:

 fabien=# \echo :VERSION_NAME
 11devel
 fabien=# \echo :VERSION_NUM
 11
 fabien=# \echo :VERSION
 PostgreSQL 11devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 
5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit
 fabien=# \echo :SERVER_VERSION_NAME
 10.1
 fabien=# \echo :SERVER_VERSION_NUM
 11

But

 fabien=# \echo :SERVER_VERSION
 :SERVER_VERSION

To get it into a variable the work around is really:

 fabien=# SELECT version() AS "SERVER_VERSION" \gset
 fabien=# \echo :SERVER_VERSION
 PostgreSQL 10.1 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 
5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit

--
Fabien
--
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: Improve bitmap costing for lossy pages

[Dilip Kumar]

On Sat, Nov 11, 2017 at 3:25 AM, Robert Haas  wrote:
> On Thu, Nov 9, 2017 at 3:55 AM, amul sul  wrote:
>> It took me a little while to understand this calculation.  You have moved 
>> this
>> code from tbm_create(), but I think you should move the following
>> comment as well:
>
> I made an adjustment that I hope will address your concern here, made
> a few other adjustments, and committed this.
>
Thanks, Robert.
-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.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] How to implement a SP-GiST index as a extension module?

[Connor Wolf]

Ok, I've managed to get my custom index working.

It's all on github here: https://github.com/fake-name/pg-spgist_hamming, if
anyone else needs a fuzzy-image searching system
that can integrate into postgresql..

It should be a pretty good basis for anyone else to use if they want to
implement a SP-GiST index too.

Thanks!

On Sun, Nov 5, 2017 at 8:10 PM, Connor Wolf  wrote:

> Never mind, it turns out the issue boiled down to me declaring the
> wrong prefixType in my config function.
>
> TL;DR - PEBKAC
>
> On Sun, Nov 5, 2017 at 1:09 AM, Connor Wolf  com> wrote:
>
>> Ok, I've got everything compiling and it installs properly, but I'm
>> running into problems that I think are either a side-effect of implementing
>> picksplit incorrectly (likely), or a bug in SP-GiST(?).
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/me
>> mcpy-sse2-unaligned.S:159
>> 159 ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such
>> file or directory.
>> (gdb) bt
>> #0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/me
>> mcpy-sse2-unaligned.S:159
>> #1  0x004ecd66 in memcpy (__len=16, __src=,
>> __dest=0x13c9dd8) at /usr/include/x86_64-linux-gnu/bits/string3.h:53
>> #2  memcpyDatum (target=target@entry=0x13c9dd8, att=att@entry=0x7fff327325f4,
>> datum=datum@entry=18445692987396472528) at spgutils.c:587
>> #3  0x004ee06b in spgFormInnerTuple (state=state@entry
>> =0x7fff327325e0, hasPrefix=, prefix=18445692987396472528,
>> nNodes=8,
>> nodes=nodes@entry=0x13bd340) at spgutils.c:741
>> #4  0x004f508b in doPickSplit (index=index@entry=0x7f2cf9de7f98,
>> state=state@entry=0x7fff327325e0, current=current@entry=0x7fff32732020,
>> parent=parent@entry=0x7fff32732040, 
>> newLeafTuple=newLeafTuple@entry=0x13b9f00,
>> level=level@entry=0, isNulls=0 '\000', isNew=0 '\000') at
>> spgdoinsert.c:913
>> #5  0x004f6976 in spgdoinsert (index=index@entry=0x7f2cf9de7f98,
>> state=state@entry=0x7fff327325e0, heapPtr=heapPtr@entry=0x12e672c,
>> datum=12598555199787281,
>> isnull=0 '\000') at spgdoinsert.c:2053
>> #6  0x004ee5cc in spgistBuildCallback (index=index@entry
>> =0x7f2cf9de7f98, htup=htup@entry=0x12e6728, values=values@entry
>> =0x7fff327321e0,
>> isnull=isnull@entry=0x7fff32732530 "", tupleIsAlive=tupleIsAlive@entry=1
>> '\001', state=state@entry=0x7fff327325e0) at spginsert.c:56
>> #7  0x00534e8d in IndexBuildHeapRangeScan
>> (heapRelation=heapRelation@entry=0x7f2cf9ddc6c8,
>> indexRelation=indexRelation@entry=0x7f2cf9de7f98,
>> indexInfo=indexInfo@entry=0x1390ad8, allow_sync=allow_sync@entry=1
>> '\001', anyvisible=anyvisible@entry=0 '\000',
>> start_blockno=start_blockno@entry=0,
>> numblocks=4294967295, callback=0x4ee573 ,
>> callback_state=0x7fff327325e0) at index.c:2609
>> #8  0x00534f52 in IndexBuildHeapScan
>> (heapRelation=heapRelation@entry=0x7f2cf9ddc6c8,
>> indexRelation=indexRelation@entry=0x7f2cf9de7f98,
>> indexInfo=indexInfo@entry=0x1390ad8, allow_sync=allow_sync@entry=1
>> '\001', callback=callback@entry=0x4ee573 ,
>> callback_state=callback_state@entry=0x7fff327325e0) at index.c:2182
>> #9  0x004eeb74 in spgbuild (heap=0x7f2cf9ddc6c8,
>> index=0x7f2cf9de7f98, indexInfo=0x1390ad8) at spginsert.c:140
>> #10 0x00535e55 in index_build 
>> (heapRelation=heapRelation@entry=0x7f2cf9ddc6c8,
>> indexRelation=indexRelation@entry=0x7f2cf9de7f98,
>> indexInfo=indexInfo@entry=0x1390ad8, isprimary=isprimary@entry=0
>> '\000', isreindex=isreindex@entry=0 '\000') at index.c:2043
>> #11 0x00536ee8 in index_create 
>> (heapRelation=heapRelation@entry=0x7f2cf9ddc6c8,
>> indexRelationName=indexRelationName@entry=0x12dd600 "int8idx_2",
>> indexRelationId=16416, indexRelationId@entry=0, relFileNode=0,
>> indexInfo=indexInfo@entry=0x1390ad8, indexColNames=indexColNames@en
>> try=0x1390f40,
>> accessMethodObjectId=4000, tableSpaceId=0,
>> collationObjectId=0x12e6b18, classObjectId=0x12e6b38, coloptions=0x12e6b58,
>> reloptions=0, isprimary=0 '\000',
>> isconstraint=0 '\000', deferrable=0 '\000', initdeferred=0 '\000',
>> allow_system_table_mods=0 '\000', skip_build=0 '\000', concurrent=0 '\000',
>> is_internal=0 '\000', if_not_exists=0 '\000') at index.c:1116
>> #12 0x005d8fe6 in DefineIndex (relationId=relationId@entry=16413,
>> stmt=stmt@entry=0x12dd568, indexRelationId=indexRelationId@entry=0,
>> is_alter_table=is_alter_table@entry=0 '\000',
>> check_rights=check_rights@entry=1 '\001', check_not_in_use=check_not_in_
>> use@entry=1 '\001', skip_build=0 '\000',
>> quiet=0 '\000') at indexcmds.c:667
>> #13 0x00782057 in ProcessUtilitySlow (pstate=pstate@entry
>> =0x12dd450, pstmt=pstmt@entry=0x12db108,
>> queryString=queryString@entry=0x12da0a0 "CREATE INDEX int8idx_2 ON
>> int8tmp_2 USING spgist ( a vptree_ops );", context=context@entry=PROCESS_
>> UTILITY_TOPLEVEL,
>> params=params@entry

Re: [HACKERS] Incorrect comment for build_child_join_rel

[Etsuro Fujita]

(2017/11/11 0:58), Robert Haas wrote:

On Fri, Nov 10, 2017 at 4:34 AM, Etsuro Fujita
  wrote:

Here is a small patch for $Subject.


Good catch.  Committed.


Thanks!

Best regards,
Etsuro Fujita


--
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] Variable substitution in psql backtick expansion

[Michael Paquier]

On Mon, Nov 13, 2017 at 5:21 AM, Tom Lane  wrote:
> Pavel Stehule  writes:
>> [ psql-server-version-2.patch ]
>
> I think this patch should be rejected.  It adds no new functionality;
> you can get the string in question with "select version()".  Moreover,
> you've been able to do that for lo these many years.  Any application
> that tried to depend on this new way of getting the string would fail
> when working with an older server or older psql.  That does not seem
> like a good property for a version check.  Also, because the string
> isn't especially machine-friendly, it's not very clear to me what the
> use-case is for an application to use it at all, rather than the other
> version formats we already provide.

+1 for rejection as version() returns PG_VERSION_STR already. It is
also already possible to define a VERSION variable psqlrc simply with
that:
\set VERSION 'version();'

+-- check consistency of SERVER_VERSION
+-- which is transmitted as GUC "server_version_raw"
+SELECT :'SERVER_VERSION' = VERSION()
+   AND :'SERVER_VERSION' = current_setting('server_version_raw')
+   AND :'SERVER_VERSION' = :'VERSION'
+   AS "SERVER_VERSION is consistent";
Not much enthusiastic with this test when thinking about cross-upgrades.
-- 
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] PATCH: psql tab completion for SELECT

[Michael Paquier]

On Mon, Nov 13, 2017 at 5:13 AM, David Fetter  wrote:
> Please add this to the upcoming (2018-01) commitfest at
> https://commitfest.postgresql.org/

You may want to scan the following thread as well:
https://www.postgresql.org/message-id/1328820579.11241.4.camel%40vanquo.pezone.net
And also you should be careful about things like WITH clauses...
-- 
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] GSoC 2017: Foreign Key Arrays

[Andreas Karlsson]

On 11/10/2017 01:47 AM, Mark Rofail wrote:

I am sorry for the late reply


There is no reason for you to be. It did not take you 6 weeks to do a 
review. :) Thanks for this new version.



== Functional review

 >1) MATCH FULL does not seem to care about NULLS in arrays. In the
example below I expected both inserts into the referring table to fail.


It seems in your example the only failed case was: INSERT INTO fk VALUES 
(NULL, '{1}');

which shouldn't work, can you clarify this?


I think that if you use MATH FULL the query should fail if you have a 
NULL in the array.



 >2) To me it was not obvious that ON DELETE CASCADE would delete
the whole rows rather than delete the members from the array, and
this kind of misunderstanding can lead to pretty bad surprises in
production. I am leaning towards not supporting CASCADE.


I would say so too, maybe we should remove ON DELETE CASCADE until we 
have supported all remaining actions.


I am leaning towards this too. I would personally be fine with a first 
version without support for CASCADE since it is not obvious to me what 
CASCADE should do.



== The @>> operator
I would argue that allocating an array of datums and building an array 
would have the same complexity


I am not sure what you mean here. Just because something has the same 
complexity does not mean there can't be major performance differences.



== Code review

 >I think the code in RI_Initial_Check() would be cleaner if you
used "CROSS JOIN LATERAL unnest(col)" rather than having unnest() in
the target list. This way you would not need to rename all columns
and the code paths for the array case could look more like the code
path for the normal case.

Can you clarify what you mean a bit more?


I think the code would look cleaner if you generate the following query:

SELECT fk.x, fk.ys FROM ONLY t2 fk CROSS JOIN LATERAL 
pg_catalog.unnest(ys) a2 (v) LEFT OUTER JOIN ONLY t1 pk ON pk.x = fk.x 
AND pk.y = a2.v WHERE [...]


rather than:

SELECT fk.k1, fk.ak2 FROM (SELECT x k1, pg_catalog.unnest(ys) k2, ys ak2 
FROM ONLY t2) fk LEFT OUTER JOIN ONLY t1 pk ON pk.x = fk.k1 AND pk.y = 
fk.k2 WHERE [...]


= New stuff

When applying the patch I got some white space warnings:

Array-ELEMENT-foreign-key-v5.3.patch:1343: space before tab in indent, 
indent with spaces.

format_type_be(oprleft), 
format_type_be(oprright;
Array-ELEMENT-foreign-key-v5.3.patch:1345: trailing whitespace.

When compiling I got an error:

ri_triggers.c: In function ‘ri_GenerateQual’:
ri_triggers.c:2693:19: error: unknown type name ‘d’
   Oid   oprcommon;d
   ^
ri_triggers.c:2700:3: error: conflicting types for ‘oprright’
   oprright = get_array_type(operform->oprleft);
   ^~~~
ri_triggers.c:2691:9: note: previous declaration of ‘oprright’ was here
   Oid   oprright;
 ^~~~
: recipe for target 'ri_triggers.o' failed

When building the documentation I got two warnings:

/usr/bin/osx:catalogs.sgml:2349:17:W: empty end-tag
/usr/bin/osx:catalogs.sgml:2350:17:W: empty end-tag

When running the tests I got a failure in element_foreign_key.

Andreas


--
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] [bug fix] postgres.exe crashes with access violation on Windows while starting up

[Michael Paquier]

On Mon, Nov 13, 2017 at 7:37 AM, Noah Misch  wrote:
> On Fri, Nov 03, 2017 at 11:10:14AM +, Michael Paquier wrote:
>> I am
>> switching the patch as ready for committer, I definitely agree that
>> you are taking the write approach here.

s/write/right/.

> Committed both patches.

Thanks for double-checking, Noah.
-- 
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] [bug fix] postgres.exe crashes with access violation on Windows while starting up

[Noah Misch]

On Sat, Oct 28, 2017 at 03:43:02PM -0700, Michael Paquier wrote:
> couldn't we envisage to just use
> malloc? pgwin32_message_to_UTF16() is called in two places in elog.c,
> and there is a full control on the error code paths.

Switching to malloc is feasible, but it wouldn't enable PostgreSQL to handle
non-ASCII messages any earlier.  Messages should be ASCII-only until the
init_locale(LC_CTYPE) call initializes MessageEncoding.  (Before that call,
pgwin32_message_to_UTF16() assumes the message is UTF8-encoded.  I've expanded
the comments slightly.  We easily comply with that restriction today.)

On Fri, Nov 03, 2017 at 11:10:14AM +, Michael Paquier wrote:
> I am
> switching the patch as ready for committer, I definitely agree that
> you are taking the write approach here.

Committed both patches.


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

[Thomas Munro]

On Sat, Nov 4, 2017 at 2:05 AM, Thomas Munro
 wrote:
> I've only tested the attached lightly on FreeBSD + OpenLDAP and
> don't know if it'll work elsewhere.

While rebasing this on top of a nearby changes, I looked into how
portable it is.  The previous version unconditionally used
ldap_initialize() instead of ldap_init() in order to be able to pass
in ldap or ldaps.  According to the man pages on my system:

   At this time, ldap_open() and ldap_init() are deprecated in favor of
   ldap_initialize(), essentially because the latter allows to specify a
   schema in the URI and it explicitly returns an error code.

But:

1.  It looks like ldap_initialize() arrived in OpenLDAP 2.4 (2007),
which means that it won't work with RHEL5's OpenLDAP 2.3.  That's a
vintage still found in the build farm.  This new version of the patch
has a configure test so it can fall back to ldap_init(), dropping
ldaps support.  That is possibly also necessary for other
implementations.

2.  Windows doesn't have ldap_initialize(), but it has
ldap_sslinit()[1] which adds an SSL boolean argument.  I've included
(but not tested) code for that.  I would need a Windows + LDAP savvy
person to help test that.  I'm not sure if it should also do an
LDAP_OPT_SSL check to see if the server forced the connection back to
plaintext as shown in the Microsoft docs[2], or if that should be
considered OK, or it should be an option.

BTW, Stephen Layland posted a patch for ldaps years ago[3].  It must
have worked some other way though, because he mentions RHEL 4 and
OpenLDAP 2.2/2.3.  Unfortunately the patch wasn't attached and the
referenced webserver has disappeared from the intertubes.

I've added this to the January Commitfest.

[1] https://msdn.microsoft.com/en-us/library/aa366996(v=vs.85).aspx
[2] https://msdn.microsoft.com/en-us/library/aa366105(v=vs.85).aspx
[3] https://www.postgresql.org/message-id/20080426010240.gs5...@68k.org

-- 
Thomas Munro
http://www.enterprisedb.com


ldaps-v3.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] Variable substitution in psql backtick expansion

[Tom Lane]

Pavel Stehule  writes:
> [ psql-server-version-2.patch ]

I think this patch should be rejected.  It adds no new functionality;
you can get the string in question with "select version()".  Moreover,
you've been able to do that for lo these many years.  Any application
that tried to depend on this new way of getting the string would fail
when working with an older server or older psql.  That does not seem
like a good property for a version check.  Also, because the string
isn't especially machine-friendly, it's not very clear to me what the
use-case is for an application to use it at all, rather than the other
version formats we already provide.

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: psql tab completion for SELECT

[David Fetter]

On Mon, Nov 06, 2017 at 05:28:55PM +1300, Edmund Horner wrote:
> Hi pgsql-hackers,
> 
> Here's a little draft patch to add *some* tab completion ability for
> SELECT in psql.  I have often missed the ability, especially with
> invocations of utility functions.
> 
> It would be nice to be able to suggest column names from the
> relevant tables in the query, but, as the SQL language puts the
> column list before the FROM clause, we have to use columns from all
> tables; I'm certainly not the first to be frustrated by this
> language feature, but we can't do much about it.
> 
> What my patch does:
> 
> For a command line with the single word SELECT, column names and
> function names will be used for completions of the next word.
> Function names have a "(" suffix added, largely because it makes
> them easier to distinguish in the completion list.
> 
> Only the first select-list item can be tab-completed; i.e. SELECT
> foo, b won't find column bar.

That can be fixed later.

> Examples:
> 
> postgres=# select rel
> relacl   relchecksrelhasindex
> relhassubclass   (etc.)
> 
> postgres=# select str
> string_to_array(  strip(strpos(

Neat!

Please add this to the upcoming (2018-01) commitfest at
https://commitfest.postgresql.org/

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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] Row Level Security Bug ?

[Joe Conway]

On 11/12/2017 10:17 AM, Andrea Adami wrote:
> if i do:
> 
> SET ROLE 'manage...@scuola-1.it '

[SELECT from table]

> i see only one row (as expected)
> 
> but when i do:

[SELECT from VIEWs]

> I see all the rows always
> 
> this way i lack all the row level security i defined
> 
> is this either a bug or it's made by design ?
> if it's made by design why ?
> Is there  a way to write view that respect the row level security ?
> For my point of view is a nonsense make a row level security that
> doesn't work with the view.

See:
https://www.postgresql.org/docs/10/static/sql-createview.html
In particular: "Access to tables referenced in the view is determined by
permissions of the view owner."

And:
https://www.postgresql.org/docs/10/static/ddl-rowsecurity.html
"Superusers and roles with the BYPASSRLS attribute always bypass the row
security system when accessing a table. Table owners normally bypass row
security as well, though a table owner can choose to be subject to row
security with ALTER TABLE ... FORCE ROW LEVEL SECURITY."

HTH,

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Fix number skipping in to_number

[Tom Lane]

Oliver Ford  writes:
> [ 0001-apply-number-v3.patch ]

I looked at this patch briefly and have a couple of comments:

* It seems entirely wrong to be matching to L_thousands_sep in the
NUM_COMMA case; that format code is by definition not locale aware,
so it should be matching to plain ',' independently of locale.

* Don't we need to fix the NUM_L (currency symbol) case in the
same manner?  (The NUM_D and NUM_S cases are handled in
NUM_numpart_from_char and seem ok at a quick glance.)

* I'm not in love with the noadd flag.  Other places in this
switch that want to skip the final increment do it with
"continue", and I think this should do likewise.

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] New gist vacuum.

[Andrey Borodin]

Hello!

> 31 янв. 2016 г., в 17:18, Alvaro Herrera  
> написал(а):
> 
> Костя Кузнецов wrote:
>> Thank you, Jeff.I reworking patch now. All // warning will 
>> be deleted.About memory consumption new version will control size 
>> of stack and will operate with map of little size because i want delete old 
>> style vacuum(now if maintenance_work_mem less than needed to build info map 
>> we use old-style vacuum with logical order).
> 
> You never got around to submitting the updated version of this patch,
> and it's been a long time now, so I'm marking it as returned with
> feedback for now.  Please do submit a new version once you have it,
> since this seems to be very useful.

I've rebased patch (see attachment), it seems to work. It still requires a bit 
of styling, docs and tests, at least.
Also, I thinks that hash table is not very good option if we have all pages 
there: we should either use array or do not fill table for every page.

If author and community do not object, I want to continue work on Konstantin's 
patch.

Best regards, Andrey Borodin.


0001-GiST-VACUUM-rebase.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

[HACKERS] Row Level Security Bug ?

[Andrea Adami]

Hello,
i have a db with a couple of tables
(enclosed the script to recreate it, please have a look before to proceed)
i enabled the row level security and all seem to work fine

if i do it (connected in as superuser like, usualy, postgres is):

select school, description, example
from schools

i can see all the rows

if i do:

SET ROLE 'manage...@scuola-1.it'

select school, description, example
from school

i see only one row (as expected)

but when i do:

select *
from _rls_test

select *
FROM _rls_test_security_barrier

select *
from _rls_test_with_check_local

select *
from _rls_test_with_check_local_cascade

I see all the rows always

this way i lack all the row level security i defined

is this either a bug or it's made by design ?
if it's made by design why ?
Is there  a way to write view that respect the row level security ?
For my point of view is a nonsense make a row level security that doesn't
work with the view.

Thanks to all the spend time to answer me.

here:
https://github.com/scuola247/postgresql
you can have a look at the complete database

Andrea Adami

===
===
===

CREATE DATABASE test
  WITH OWNER = postgres
   ENCODING = 'UTF8'
   TABLESPACE = pg_default
   CONNECTION LIMIT = -1;


CREATE SEQUENCE public.pk_seq
  INCREMENT 1
  MINVALUE 1
  MAXVALUE 9223372036854775807
  START 736220
  CACHE 1;


CREATE TABLE public.schools
(
  school bigint NOT NULL DEFAULT nextval('pk_seq'::regclass), -- Uniquely
identifies the table row
  description character varying(160) NOT NULL, -- Description for the school
  processing_code character varying(160) NOT NULL, -- A code that identify
the school on the government information system
  mnemonic character varying(30) NOT NULL, -- Short description to be use
as code
  example boolean NOT NULL DEFAULT false, -- It indicates that the data
have been inserted to be an example of the use of the data base
  behavior bigint, -- Indicates the subject used for the behavior
  CONSTRAINT schools_pk PRIMARY KEY (school),
  CONSTRAINT schools_uq_description UNIQUE (description),
  CONSTRAINT schools_uq_mnemonic UNIQUE (mnemonic),
  CONSTRAINT schools_uq_processing_code UNIQUE (processing_code, example)
);

-- Index: public.schools_fk_behavior

CREATE INDEX schools_fk_behavior
  ON public.schools
  USING btree
  (behavior);


CREATE TABLE public.usenames_schools
(
  usename_school bigint NOT NULL DEFAULT nextval('pk_seq'::regclass), --
Unique identification code for the row
  usename name NOT NULL, -- The session's usename
  school bigint NOT NULL, -- School enabled for the the usename
  CONSTRAINT usenames_schools_pk PRIMARY KEY (usename_school),
  CONSTRAINT usenames_schools_fk_school FOREIGN KEY (school)
  REFERENCES public.schools (school) MATCH SIMPLE
  ON UPDATE CASCADE ON DELETE CASCADE,
  CONSTRAINT usenames_schools_uq_usename_school UNIQUE (usename, school) --
Foe every usename one school can be enabled only one time
);

-- Index: public.usenames_schools_fx_school

CREATE INDEX usenames_schools_fx_school
  ON public.usenames_schools
  USING btree
  (school);

  CREATE OR REPLACE VIEW public._rls_test AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;


CREATE OR REPLACE VIEW public._rls_test_security_barrier WITH
(security_barrier=true) AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;

CREATE OR REPLACE VIEW public._rls_test_with_check_local WITH
(check_option=local) AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;

CREATE OR REPLACE VIEW public._rls_test_with_check_local_cascade WITH
(check_option=cascaded) AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;

-- now same data
-- now same data
-- now same data

INSERT INTO
public.schools(school,description,processing_code,mnemonic,example) VALUES
('289610','Istituto comprensivo "Voyager"','ZZIC1Z','IC
VOYAGER','t');
INSERT INTO
public.schools(school,description,processing_code,mnemonic,example) VALUES
('20','Istituto Tecnico Tecnologico "Leonardo da
Vinci"','ZZITTZ','ITT DAVINCI','t');
INSERT INTO
public.schools(school,description,processing_code,mnemonic,example) VALUES
('10','Istituto comprensivo ''Andromeda''','ZZIC8Z','IC
ANDROMEDA','t');


INSERT INTO public.usenames_schools(usename_school,usename,school) VALUES
('7266330','manage...@scuola-1.it','10');


-- THEN ENABLE ROW LEVEL SECURITY
-- THEN ENABLE ROW LEVEL SECURITY
-- THEN ENABLE ROW LEVEL SECURITY


  ALTER TABLE usenames_schools ENABLE ROW LEVEL SECURITY;

 ALTER TABLE schools ENABLE ROW LEVEL SECURITY;

CREATE POLICY usenames_schools_pl_usename ON usenames_schools TO public
 USING (usename = current_user)
  WITH CHECK (usename = current_user);

CREATE POLICY schools_pl_school ON schools TO

Re: [HACKERS] PSA: don't be in a hurry to update to XCode 9.0

[Tom Lane]

Dave Cramer  writes:
> Did you ever find a solution to this without updating ?

No.  I filed a bug report which Apple seems uninterested in, perhaps
not too surprisingly.

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] PSA: don't be in a hurry to update to XCode 9.0

[Dave Cramer]

Tom,

Did you ever find a solution to this without updating ?

Dave Cramer

da...@postgresintl.com
www.postgresintl.com

On 21 September 2017 at 13:01, Dave Cramer  wrote:

> Too late I just stumbled over this as well!
>
> Dave Cramer
>
> da...@postgresintl.com
> www.postgresintl.com
>
> On 20 September 2017 at 14:34, Tom Lane  wrote:
>
>> It seems to install some libraries that depend on
>> /usr/lib/system/libsystem_darwin.dylib, which doesn't exist.
>> Googling suggests it will be there in macOS 10.13,
>> but on Sierra or earlier, you're outta luck.
>>
>> It looks like the only directly-affected PG dependency is
>> libxml; if you leave out --with-libxml you can still build.
>>
>> I found this out the hard way on longfin's host, so I've
>> temporarily removed --with-libxml from that animal's
>> configuration to restore it to service.  I trust I'll be
>> able to re-enable that after 10.13 comes out.
>>
>> 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] WIP: Covering + unique indexes.

[Andrey Borodin]

Hi!
+1 for pushing this. I'm really looking forward to see this in 11.

> 31 окт. 2017 г., в 13:21, Anastasia Lubennikova 
>  написал(а):
> 
> Updated version is attached. It applies to the commit 
> e4fbf22831c2bbcf032ee60a327b871d2364b3f5.
> The first patch patch contains changes in general index routines
> and the second one contains btree specific changes.
> 
> This version contains fixes of the issues mentioned in the thread above and 
> passes all existing tests.
> But still it requires review and testing, because the merge was quite uneasy.
> Especially I worry about integration with partitioning. I'll add some more 
> tests in the next message.

I've been doing benchmark tests a year ago, so I skip this part in this review.

I've done some stress tests with pgbench, replication etc. Everything was fine 
until I plugged in amcheck.
If I create cluster with this [0] and then 
./pgbench -i -s 50

create index on pgbench_accounts (abalance) include (bid,aid,filler); 
create extension amcheck;

--and finally 
SELECT bt_index_check(c.oid), c.relname, c.relpages
FROM pg_index i
JOIN pg_opclass op ON i.indclass[0] = op.oid
JOIN pg_am am ON op.opcmethod = am.oid
JOIN pg_class c ON i.indexrelid = c.oid
JOIN pg_namespace n ON c.relnamespace = n.oid
WHERE am.amname = 'btree' AND n.nspname = 'public'
AND c.relpersistence != 't'
AND i.indisready AND i.indisvalid
ORDER BY c.relpages DESC LIMIT 100;
--just copypasted from amcheck docs with minor corrections

Postgres crashes: 
TRAP: FailedAssertion("!(((const void*)(&isNull) != ((void*)0)) && 
(scankey->sk_attno) > 0)", File: "nbtsearch.c", Line: 466)

May be I'm doing something wrong or amcheck support will go with different 
patch?

Few minor nitpicks:
0. PgAdmin fails to understand what is going on [1] . It is clearly problem of 
PgAdmin, pg_dump works as expected.
1. ISTM index_truncate_tuple() can be optimized. We only need to reset tuple 
length and infomask. But this should not be hotpath, anyway, so I propose 
ignoring this for current version.
2. I've done grammarly checking :) This comma seems redundant [2]
I don't think something of these items require fixing.

Thanks for working on this, I believe it is important.

Best regards, Andrey Borodin.

[0] https://github.com/x4m/pgscripts/blob/master/install.sh
[1] https://yadi.sk/i/ro9YKFqo3PcwFT
[2] 
https://github.com/x4m/postgres_g/commit/657c28952d923d8c150e6cabb3bdcbbc44a641b6?diff=unified#diff-640baf2937029728a8d51cccd554c2eeR1291

-- 
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] A GUC to prevent leader processes from running subplans?

[Thomas Munro]

On Sun, Nov 12, 2017 at 8:51 PM, Amit Kapila  wrote:
> On Sun, Nov 12, 2017 at 9:18 AM, Thomas Munro
>  wrote:
>> How about parallel_leader_participation = on|off?  The attached
>> version has it that way, and adds regression tests to exercise on, off
>> and off-but-couldn't-start-any-workers for both kinds of gather node.
>>
>> I'm not sure why node->need_to_rescan is initialised by both
>> ExecGatherInit() and ExecGather().  Only the latter's value matters,
>> right?
>>
>
> I don't see anything like need_to_rescan in the GatherState node.  Do
> you intend to say need_to_scan_locally?  If yes, then I think whatever
> you said is right.

Right, that's what I meant to write.  Thanks.

>> I've added this to the January Commitfest.
>>
>
> +1 to this idea.  Do you think such an option at table level can be
> meaningful?  We have a parallel_workers as a storage option for
> tables, so users might want leader to participate in parallelism only
> for some of the tables.

I'm not sure.  I think the reason for turning it off (other than
developer testing) would be that the leader is getting tied up doing
work that takes a long time (sorting, hashing, aggregating) and that's
causing the workers to be blocked because their output queue is full.
I think that type of behaviour comes from certain plan types, and it
probably wouldn't make sense to associate this behaviour with the
tables you're scanning.

-- 
Thomas Munro
http://www.enterprisedb.com


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