Re: [HACKERS] proposal: Support Unicode host variable in ECPG

[Michael Paquier]

On Thu, Oct 26, 2017 at 12:29 PM, Jing Wang  wrote:
> I would like to provide the patch file that ECPG can support the utf16/utf32
> unicode host variable type. This feature is used to compatible the same
> feature in Oracle's. Please find Oracle's feature in the [1].
>
> [1] https://docs.oracle.com/database/121/LNPCC/pc_05adv.htm#LNPCC3273

The core of the patch is roughly 800 lines, and it adds 20k lines of
tests, which is large to digest.

+PQGetEncodingFromPGres173
+PQGetEncodingFromPGconn   174
+PQGetEncodingName 175
+PQGenEncodingMaxLen   176
This adds four undocumented APIs to libpq. And why not using client_encoding?

The patch does not apply anymore and needs a rebase. There are also a
bunch of whitespace errors, no documentation in the patch, and it
seems to me that this patch introduces many concepts so it could be
broken down into many individual steps. Finally, I strongly recommend
that you review other's patches. You have two patches, including this
one, registered into this commit fest but your name is showing nowhere
as a reviewer. The patch you are proposing here is large, the usual
recommendation being to review one patch of equal difficulty for each
patch sent to keep the inflow and outflow of patches balanced.

I am adding Michael Meskes to this thread, he is the maintainer of
ECPG so perhaps he would be interested in what you have here.

Thanks.
-- 
Michael

Re: [HACKERS] [PATCH] Incremental sort

[Peter Geoghegan]

On Mon, Nov 20, 2017 at 3:34 PM, Alexander Korotkov
 wrote:
> Thank you very much for review.  I really appreciate this topic gets
> attention.  Please, find next revision of patch in the attachment.

I would really like to see this get into v11. This is an important
patch, that has fallen through the cracks far too many times.

-- 
Peter Geoghegan

Re: [HACKERS] pgbench regression test failure

[Fabien COELHO]

Hello Tom,


2. ISTM that we should report that 100% of the transactions were
above the latency limit, not 33%; that is, the appropriate base
for the "number of transactions above the latency limit" percentage
is the number of actual transactions not the number of scheduled
transactions.



Hmmm. Allow me to disagree.


I dunno, it just looks odd to me that when we've set up a test case in
which every one of the transactions is guaranteed to exceed the latency
limit, that it doesn't say that they all did.  I don't particularly buy
your assumption that the percentages should sum.


This is a side effect. The reason for me is that the user asked for some 
transactions, and the results should be given relative to what was asked.



Anybody else have an opinion there?


Good question.


I also noticed that if I specify "-f sleep-100.sql" more than once,
the per-script TPS reports are out of line.  This is evidently because
that calculation isn't excluding skipped xacts; but if we're going to
define tps as excluding skipped xacts, surely we should do so there too.



I do not think that we should exclude skipped xacts.


Uh ... why not?


Because I totally misinterpreted your sentence.

Indeed, the skipped transactions needs to be substracted from the count.
This is yet another bug.


but I find that unduly optimistic.  It should really read more like
"if no transactions were executed, at best we'll get some platform-
dependent spelling of NaN.  At worst we'll get a SIGFPE."



Hmmm. Alas you must be right about spelling. There has been no report of
SIGFPE issue, so I would not bother with that.


The core issue here really is that you're assuming IEEE float arithmetic.
We have not gone as far as deciding that Postgres will only run on IEEE
hardware, and I don't want to start in pgbench, especially not in
seldom-exercised corner cases.


Hmmm. It has already started for some years without complaint. Do as you 
feel about NaN. I can only say that I do not like much having zero to 
stand for undefined.


--
Fabien.

Re: [HACKERS] CUBE seems a bit confused about ORDER BY

[Alexander Korotkov]

On Mon, Nov 20, 2017 at 1:59 PM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> On Mon, Nov 20, 2017 at 4:09 AM, Tomas Vondra <
> tomas.von...@2ndquadrant.com> wrote:
>
>> Seems fine to me, although perhaps it should be split into two parts.
>> One with the cube_coord_llur fixes, and then g_cube_distance changes
>> adding support for negative coordinates.
>
>
> Thank you for your feedback.  I'll split this patch into two.
>

Please, find two patches attached.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


0001-cube-knn-fix.patch
Description: Binary data


0002-cube-knn-negative-coordinate.patch
Description: Binary data

Re: Using isatty() on WIN32 platform

[Tom Lane]

=?UTF-8?Q?Mart=c3=adn_Marqu=c3=a9s?=  writes:
> While following suggestions from Arthur Zakirov on a patch for
> pg_basebackup I found that we are using isatty() in multiple places, but
> we don't distinguish the WIN32 code which should use _isatty() as per [1].

I dunno, [1] looks like pure pedantry to me.  Unless they intend to stop
conforming to POSIX at all, they aren't going to be able to remove the
isatty() spelling.

Moreover, I don't think that isatty() is the only function name at issue.
I found this at:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/compatibility

The C++ standard reserves names that begin with an underscore in the
global namespace to the implementation. Because the POSIX functions
are in the global namespace, but are not part of the standard C
runtime library, the Microsoft-specific implementations of these
functions have a leading underscore. For portability, the UCRT also
supports the default names, but the Visual C++ compiler issues a
deprecation warning when code that uses them is compiled. Only the
default POSIX names are deprecated, not the functions. To suppress the
warning, define _CRT_NONSTDC_NO_WARNINGS before including any headers
in code that uses the original POSIX names.

If you're seeing warnings from use of isatty(), I'd be inclined to think
about dealing with it by adding #define _CRT_NONSTDC_NO_WARNINGS,
rather than trying to individually #define every affected function.

> Attached is a patch for src/include/ports/win32.h

FWIW, if we do adopt that, I think it should now go into win32_port.h.
In either case, though, seems like it might be problematic to have
such a #define before including whatever file Microsoft's definition
lives in.

regards, tom lane

Re: [HACKERS] pg_basebackup --progress output for batch execution

[Martín Marqués]

El 09/11/17 a las 09:29, Arthur Zakirov escribió:
> Hello,
> 
> On Sun, Oct 01, 2017 at 04:49:17PM -0300, Martin Marques wrote:
>> Updated patch with documentation of the new option.
>>
> 
> I have checked the patch.
> The patch is applied and compiled correctly without any errors. Tests passed.
> The documentation doesn't have errors too.
> 
> 
> I have a little suggestion. Maybe insert new line without any additional 
> parameters? We can check that stderr is not terminal using isatty().
> 
> The code could become:
> 
> if (!isatty(fileno(stderr)))
>   fprintf(stderr, "\n");
> else
>   fprintf(stderr, "\r");
> 
> Also it could be good to not insert new line after progress:
> 
> if (showprogress)
> {
>   progress_report(PQntuples(res), NULL, true);
>   /* if (!batchmode) */
>   /* or */
>   if (isatty(fileno(stderr)))
>   fprintf(stderr, "\n");  /* Need to move to next line */
> }


New version of patch, without the --batch-mode option and using isatty()

I send in a separate thread a proposal to make isatty() be defined as
_isatty() in windows.


-- 
Martín Marquéshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
new file mode 100644
index a8715d9..575a4f9
*** a/src/bin/pg_basebackup/pg_basebackup.c
--- b/src/bin/pg_basebackup/pg_basebackup.c
*** progress_report(int tablespacenum, const
*** 811,817 
  totaldone_str, totalsize_str, percent,
  tablespacenum, tablespacecount);
  
! 	fprintf(stderr, "\r");
  }
  
  static int32
--- 811,820 
  totaldone_str, totalsize_str, percent,
  tablespacenum, tablespacecount);
  
! 	if (!isatty(fileno(stderr)))
! 		fprintf(stderr, "\n");
! 	else
! 		fprintf(stderr, "\r");
  }
  
  static int32
*** BaseBackup(void)
*** 1796,1802 
  progname);
  
  	if (showprogress && !verbose)
! 		fprintf(stderr, "waiting for checkpoint\r");
  
  	basebkp =
  		psprintf("BASE_BACKUP LABEL '%s' %s %s %s %s %s %s",
--- 1799,1811 
  progname);
  
  	if (showprogress && !verbose)
! 	{
! 		fprintf(stderr, "waiting for checkpoint");
! 		if (isatty(fileno(stderr)))
! 			fprintf(stderr, "\n");
! 		else
! 			fprintf(stderr, "\r");
! 	}
  
  	basebkp =
  		psprintf("BASE_BACKUP LABEL '%s' %s %s %s %s %s %s",
*** BaseBackup(void)
*** 1929,1935 
  	if (showprogress)
  	{
  		progress_report(PQntuples(res), NULL, true);
! 		fprintf(stderr, "\n");	/* Need to move to next line */
  	}
  
  	PQclear(res);
--- 1938,1945 
  	if (showprogress)
  	{
  		progress_report(PQntuples(res), NULL, true);
! 		if (isatty(fileno(stderr)))
! 			fprintf(stderr, "\n");	/* Need to move to next line */
  	}
  
  	PQclear(res);

Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

[Andres Freund]

Hi,

On 2017-11-13 19:03:41 -0800, Andres Freund wrote:
> Hi,
> 
> On 2017-11-03 07:53:30 -0700, Andres Freund wrote:
> > Here's that patch.  I've stared at this some, and Robert did too. Robert
> > mentioned that the commit message might need some polish and I'm not
> > 100% sure about the error message texts yet.
> > 
> > I'm not yet convinced that the new elog in vacuumlazy can never trigger
> > - but I also don't think we want to actually freeze the tuple in that
> > case.
> 
> I'm fairly sure it could be triggered, therefore I've rewritten that.
> 
> I've played around quite some with the attached patch. So far, after
> applying the second patch, neither VACUUM nor VACUUM FULL / CLUSTER make
> the situation worse for already existing corruption. HOT pruning can
> change the exact appearance of existing corruption a bit, but I don't
> think it can make the corruption meaningfully worse.  It's a bit
> annoying and scary to add so many checks to backbranches but it kinda
> seems required.  The error message texts aren't perfect, but these are
> "should never be hit" type elog()s so I'm not too worried about that.
> 
> 
> Please review!

Ping? Alvaro, it'd be good to get some input here.

- Andres

Re: [PATCH] Porting small OpenBSD changes.

[David Fetter]

On Mon, Nov 20, 2017 at 06:57:47PM +, David CARLIER wrote:
> On 20 November 2017 at 18:49, Tom Lane  wrote:
> 
> > David CARLIER  writes:
> > > - Reducing OpenBSD postfgresql maintainer internal changes bookeeping if
> > > those small changes make sense for the PostgreSQL developers.
> >
> > Hm.  The s_lock.c change is surely fine if OpenBSD maintainers say it is.
> >
> > Not sure about adding Motorola 88K support to s_lock.h ... is anybody
> > really still interested in that platform?
> 
> 
> Yes there is :-)

Any chance of a buildfarm http://www.pgbuildfarm.org/ member or two
with this architecture?

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

Re: [HACKERS] [PATCH] A hook for session start

[Fabrízio de Royes Mello]

On Mon, Nov 20, 2017 at 2:56 PM, Tom Lane  wrote:
>
> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  writes:
> > typedef enum
> > {
> > ClientBackendProcess = -1,
> > CheckerProcess = 0,
> > BootstrapProcess,
>
> Uh, why would you do that (start from -1)?  It makes it impossible to
> build an array indexed by the enum, which might be useful --- converting
> enum values to strings is one obvious application.  Breaks your
> "NUM_PROCTYPES" thing, too.
>

I agree... I just copy and paste AuxProcType with some kind of
generalization.


> What I'd do is probably
>
> UnknownProcess = 0,
> PostmasterProc,
> StandaloneBackendProc,
> ClientBackendProc,
> BootstrapProc,
> ...
> NUM_PROCTYPES/* Must be last! */
>
> The value of reserving "UnknownProcess = 0" is that a freshly started
> process would be correctly not-identified if the variable starts out 0.
> (I'd be rather tempted to teach fork_process to reset it to 0 immediately
> after forking, too, so that postmaster children couldn't inadvertently
> retain the PostmasterProc setting.)
>

Makes sense...


> I'm not entirely sure whether standalone backends ought to get their
> own code, or whether it's better for them to share ClientBackendProc.
> It's something we'd have to work out as we went through the code
> making the patch.  How many places would want to distinguish, versus
> how many would have to test for two values?
>

Maybe for the first version we use just "ClientBackendProc" and refactor
all the code necessary to generalize AuxProcType. Then we can step into
into. Seems reasonable?


> > extern ProcType MyProcType;
>
> "PGProcType", maybe?  "ProcType" alone seems pretty generic.
> "ServerProcType" is another possibility for the typedef name,
> to emphasize that what we are classifying is the postmaster
> and its children.
>

"ServerProcType" seems a good name.


Due to some "Blackfriday" commitments I'll be able to work again with this
patch on next week.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: [PATCH] Porting small OpenBSD changes.

[David CARLIER]

On 20 November 2017 at 18:49, Tom Lane  wrote:

> David CARLIER  writes:
> > - Reducing OpenBSD postfgresql maintainer internal changes bookeeping if
> > those small changes make sense for the PostgreSQL developers.
>
> Hm.  The s_lock.c change is surely fine if OpenBSD maintainers say it is.
>
> Not sure about adding Motorola 88K support to s_lock.h ... is anybody
> really still interested in that platform?


Yes there is :-)


> OTOH, we still have M68K
> and VAX stanzas in that file, so I suppose it's silly to complain
> about 88K.  A bigger issue is that I wonder whether that code has
> ever been tested: it does not look to me like the __asm__ call is
> even syntactically correct.  There should be colons in it.
>
>
True :-) corrected. Thanks.


> regards, tom lane
>


0001-PATCH-1-1-Porting-OpenBSD-internal-changes.patch
Description: Binary data

Re: [PATCH] Porting small OpenBSD changes.

[Tom Lane]

David CARLIER  writes:
> - Reducing OpenBSD postfgresql maintainer internal changes bookeeping if
> those small changes make sense for the PostgreSQL developers.

Hm.  The s_lock.c change is surely fine if OpenBSD maintainers say it is.

Not sure about adding Motorola 88K support to s_lock.h ... is anybody
really still interested in that platform?  OTOH, we still have M68K
and VAX stanzas in that file, so I suppose it's silly to complain
about 88K.  A bigger issue is that I wonder whether that code has
ever been tested: it does not look to me like the __asm__ call is
even syntactically correct.  There should be colons in it.

regards, tom lane

Re: [HACKERS] pgbench regression test failure

[Tom Lane]

Fabien COELHO  writes:
>> 1. The per-script stats shouldn't be printed at all if there's
>> only one script.  They're redundant with the overall stats.

> Indeed.
> I think the output should tend to be the same for possible automatic 
> processing, whether there is one script or more, even at the price of some 
> redundancy.
> Obviously this is highly debatable.

I think that ship has already sailed.  It's certainly silly that the
code prints *only* per-script latency stats, and not all the per-script
stats, when there's just one script.  To me the answer is to make the
latency stats conform to the rest, not make the printout even more
redundant.  None of this output was designed for machine-friendliness.

(Maybe there is an argument for a "--machine-readable-output" switch
that would dump the data in some more-machine-friendly format.  Though
I'm sure we'd have lots of debates about exactly what that is...)

>> 2. ISTM that we should report that 100% of the transactions were
>> above the latency limit, not 33%; that is, the appropriate base
>> for the "number of transactions above the latency limit" percentage
>> is the number of actual transactions not the number of scheduled
>> transactions.

> Hmmm. Allow me to disagree.

I dunno, it just looks odd to me that when we've set up a test case in
which every one of the transactions is guaranteed to exceed the latency
limit, that it doesn't say that they all did.  I don't particularly buy
your assumption that the percentages should sum.  Anybody else have an
opinion there?

>> I also noticed that if I specify "-f sleep-100.sql" more than once,
>> the per-script TPS reports are out of line.  This is evidently because
>> that calculation isn't excluding skipped xacts; but if we're going to
>> define tps as excluding skipped xacts, surely we should do so there too.

> I do not think that we should exclude skipped xacts.

Uh ... why not?

>> I'm also still exceedingly unhappy about the NaN business.
>> You have this comment in printSimpleStats:
>> /* print NaN if no transactions where executed */
>> but I find that unduly optimistic.  It should really read more like
>> "if no transactions were executed, at best we'll get some platform-
>> dependent spelling of NaN.  At worst we'll get a SIGFPE."

> Hmmm. Alas you must be right about spelling. There has been no report of 
> SIGFPE issue, so I would not bother with that.

The core issue here really is that you're assuming IEEE float arithmetic.
We have not gone as far as deciding that Postgres will only run on IEEE
hardware, and I don't want to start in pgbench, especially not in
seldom-exercised corner cases.

regards, tom lane

Re: [HACKERS] pg_basebackup --progress output for batch execution

[Martín Marqués]

El 14/11/17 a las 06:25, Arthur Zakirov escribió:
> On Fri, Nov 10, 2017 at 10:32:23AM -0300, Martín Marqués wrote:
>> An example where using isatty() might fail is if you run pg_basebackup
>> from a tty but redirect the output to a file, I believe that in that
>> case isatty() will return true, but it's very likely that the user
>> might want batch mode output.
> 
> Sorry if I misunderstood you. I think this can happen if you redirect only 
> standard output (stdout) to a file.

Don't be sorry. It was me who misunderstood how isatty() worked. Shame
on me. :(

I will iterate again with a new patch which has your ideas from before.

P.D.: But I'm going to first open a new thread on WIN32 compatibility of
isatty()

-- 
Martín Marquéshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] CLUSTER command progress monitor

[Tom Lane]

Antonin Houska  writes:
> Robert Haas  wrote:
>> These two phases overlap, though. I believe progress reporting for
>> sorts is really hard.

> Whatever complexity is hidden in the sort, cost_sort() should have taken it
> into consideration when called via plan_cluster_use_sort(). Thus I think that
> once we have both startup and total cost, the current progress of the sort
> stage can be estimated from the current number of input and output
> rows. Please remind me if my proposal appears to be too simplistic.

Well, even if you assume that the planner's cost model omits nothing
(which I wouldn't bet on), its result is only going to be as good as the
planner's estimate of the number of rows to be sorted.  And, in cases
where people actually care about progress monitoring, it's likely that
the planner got that wrong, maybe horribly so.  I think it's a bad idea
for progress monitoring to depend on the planner's estimates in any way
whatsoever.

regards, tom lane

Re: [HACKERS] [PATCH] A hook for session start

[Tom Lane]

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  writes:
> typedef enum
> {
> ClientBackendProcess = -1,
> CheckerProcess = 0,
> BootstrapProcess,

Uh, why would you do that (start from -1)?  It makes it impossible to
build an array indexed by the enum, which might be useful --- converting
enum values to strings is one obvious application.  Breaks your
"NUM_PROCTYPES" thing, too.

What I'd do is probably

UnknownProcess = 0,
PostmasterProc,
StandaloneBackendProc,
ClientBackendProc,
BootstrapProc,
...
NUM_PROCTYPES/* Must be last! */

The value of reserving "UnknownProcess = 0" is that a freshly started
process would be correctly not-identified if the variable starts out 0.
(I'd be rather tempted to teach fork_process to reset it to 0 immediately
after forking, too, so that postmaster children couldn't inadvertently
retain the PostmasterProc setting.)

I'm not entirely sure whether standalone backends ought to get their
own code, or whether it's better for them to share ClientBackendProc.
It's something we'd have to work out as we went through the code
making the patch.  How many places would want to distinguish, versus
how many would have to test for two values?

> extern ProcType MyProcType;

"PGProcType", maybe?  "ProcType" alone seems pretty generic.
"ServerProcType" is another possibility for the typedef name,
to emphasize that what we are classifying is the postmaster
and its children.

regards, tom lane

Re: [HACKERS] Custom compression methods

[Ildus Kurbangaliev]

On Mon, 20 Nov 2017 16:29:11 +0100
Tomas Vondra  wrote:

> On 11/20/2017 04:21 PM, Евгений Шишкин wrote:
> > 
> >   
> >> On Nov 20, 2017, at 18:18, Tomas Vondra
> >>  >> > wrote:
> >>
> >>
> >> I don't think we need to do anything smart here - it should behave
> >> just like dropping a data type, for example. That is, error out if
> >> there are columns using the compression method (without CASCADE),
> >> and drop all the columns (with CASCADE).  
> > 
> > What about instead of dropping column we leave data uncompressed?
> >   
> 
> That requires you to go through the data and rewrite the whole table.
> And I'm not aware of a DROP command doing that, instead they just drop
> the dependent objects (e.g. DROP TYPE, ...). So per PLOS the DROP
> COMPRESSION METHOD command should do that too.
> 
> But I'm wondering if ALTER COLUMN ... SET NOT COMPRESSED should do
> that (currently it only disables compression for new data).

If the table is big, decompression could take an eternity. That's why i
decided to only to disable it and the data could be decompressed using
compression options.

My idea was to keep compression options forever, since there will not
be much of them in one database. Still that requires that extension is
not removed.

I will try to find a way how to recompress data first in case it moves
to another table.

-- 
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Re: Jsonb transform for pl/python

[Aleksander Alekseev]

Hi Anthony,

> thank you for your review. I took your comments into account in the
> third version of the patch. In the new version, I've added all the
> tests you asked for. The interesting thing is that:
> 1. set or any other non-jsonb-transformable object is transformed into
> string and added to jsonb as a string. 

Well frankly I very much doubt that this:

```
+-- set -> jsonb
+CREATE FUNCTION test1set() RETURNS jsonb
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+x = set()
+x.add(1)
+x.add("string")
+x.add(None)
+return x
+$$;
+SELECT test1set();
+  test1set  
+
+ "set([1, 'string', None])"
+(1 row)
```

... is an expected and valid behavior. If user tries to transform a
set() to JSONB this is most likely a mistake since there is no standard
representation of a set() in JSONB. I believe we should rise an error in
this case instead of generating a string. Besides user can expect that
such string can be transformed back to set() which doesn't sound like a
good idea either.

If necessary, user can just transform a set() to a list():

```
>>> x = set([1,2,3,4])
>>> x
{1, 2, 3, 4}
>>> list(x)
[1, 2, 3, 4]
```

BTW I just recalled that Python supports complex numbers out-of-the box
and that range and xrange are a separate types too:

```
>>> 1 + 2j
(1+2j)
>>> range(3)
range(0, 3)
>>> type(range(3))

```

I think we should add all this to the tests as well. Naturally complex
numbers can't be represented in JSON so we should rise an error if user
tries to transform a complex number to JSON. I'm not that sure regarding
ranges though. Probably the best solution will be to rise and error in
this case as well just to keep things consistent.

> +ERROR:  jsonb doesn't support inf type yet

I would say this error message is too informal. How about something more
like "Infinity can't be represented in JSONB"?

> 2. couldn't find a solution of working with "inf": this extension
> troughs exception if python generates a number called "inf" and returns
> it, but if we pass a very large integer into a function, it works fine
> with the whole number. This situation can be seen in tests.
> 
> I've added tests of large numerics which weights quite heavy. So,
> please find it in compressed format in attachments.

I'm afraid that tests fail on Python 3:

```
  SELECT test1set();
 test1set
  ---
!  "{None, 1, 'string'}"
  (1 row)
  
  DROP EXTENSION plpython3u CASCADE;
--- 296,302 
  SELECT test1set();
 test1set
  ---
!  "{1, None, 'string'}"
  (1 row)
  
  DROP EXTENSION plpython3u CASCADE
```

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

[PATCH] Porting small OpenBSD changes.

[David CARLIER]

Hi,

Compilation pass, make check passes.

Motivations :

- Reducing OpenBSD postfgresql maintainer internal changes bookeeping if
those small changes make sense for the PostgreSQL developers.

Hope it is good.

Thanks in advance.

Kind regards.


0001-PATCH-1-1-Porting-OpenBSD-internal-changes.patch
Description: Binary data

Re: [COMMITTERS] pgsql: Add hash partitioning.

[amul sul]

On Sat, Nov 18, 2017 at 1:19 AM, Robert Haas  wrote:
> On Thu, Nov 16, 2017 at 9:37 AM, amul sul  wrote:
>> Fixed in the 001 patch.
>>
>> IMHO, this function is not meant for a user, so that instead of ereport() 
>> cant
>> we simply return false?  TWIW, I have attached 003 patch which replaces all
>> erepots() by return false.
>
> I don't think just returning false is very helpful behavior, because
> the user may not realize that the problem is that the function is
> being called incorrectly rather than that the call is correct and the
> answer is false.
>
> I took your 0001 patch and made extensive modifications to it.  I
> replaced your regression tests from 0002 with a new set that I wrote
> myself.  The result is attached here.  This version makes different
> decisions about how to handle the various problem cases than you did;
> it returns NULL for a NULL input or an OID for which no relation
> exists, and throws specific error messages for the other cases,
> matching the parser error messages that CREATE TABLE would issue where
> possible, but with a different error code.  It also checks that the
> types match (which your patch did not, and which I'm fairly sure could
> crash the server), makes the function work when invoked using the
> explicit VARIADIC syntax (which seems fairly useless here but there's
> no in-core precedent for variadic function which doesn't support that
> case), and fixes the function header comment to describe the behavior
> we committed rather than the older behavior you had in earlier patch
> versions.
>
> As far as I can tell, this should nail things down pretty tight, but
> it would be great if someone can try to find a case where it still
> breaks.
>

Thanks for fixing this function.  Patch looks good to me, except column number
in the following errors message should to be 2.

354 +SELECT satisfies_hash_partition('mchash'::regclass, 2, 1,
NULL::int, NULL::int);
355 +ERROR:  column 1 of the partition key has type "text", but
supplied value is of type "integer"

Same at the line # 374 & 401 in the patch.

Regards,
Amul

Unicode or local language

[Lelisa Diriba]

unicode=# SET CLIENT_ENCODING TO UNICODE;
SET
unicode=# show client_encoding;
 client_encoding
-
 UTF8
(1 row)
insert into person(name) values('ሳምሶን');when i insert From PGAdmin and when
i want to SELECT FROM  SQL SHELL(PSQL) i doesn't the normal value.how psql
support UNICODE character?
unicode=# select * from person;( from SQL SHELL)
 ሳምሶን
(1 rows)
select * from person;(from PGAdmin)
ሳምሶን
(1 rows)
The Language i use is "Amharic language" which is From Africa (Ethiopia
country)
I want use the local language Amharic from  SQL SHELL,
How i use the Local Language  from SQL SHELL?

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On 19 November 2017 at 16:13, Arthur Zakirov 
wrote:
>
> I think it is time to mark the patch as "Ready for Commiter". I've
> marked it.

Good, thank you for the comprehensive review.

Failed to delete old ReorderBuffer spilled files

[atorikoshi]

Hi,

I put many queries into one transaction and made ReorderBuffer spill
data to disk, and sent SIGKILL to postgres before the end of the
transaction.

After starting up postgres again, I observed the files spilled to
data wasn't deleted.

I think these files should be deleted because its transaction was no
more valid, so no one can use these files.


Below is a reproduction instructions.


1. Create table and publication at publiser.

  @pub =# CREATE TABLE t1(
  id INT PRIMARY KEY,
  name TEXT);

  @pub =# CREATE PUBLICATION pub FOR TABLE t1;

2. Create table and subscription at subscriber.

  @sub =# CREATE TABLE t1(
  id INT PRIMARY KEY,
  name TEXT
  );

  @sub =# CREATE SUBSCRIPTION sub
  CONNECTION 'host=[hostname] port=[port] dbname=[dbname]'
  PUBLICATION pub;

3. Put many queries into one transaction.

  @pub =# BEGIN;
INSERT INTO t1
SELECT
  i,
  'aa'
FROM
generate_series(1, 100) as i;

4. Then we can see spilled files.

  @pub $ ls -1 ${PGDATA}/pg_replslot/sub/
state
xid-561-lsn-0-100.snap
xid-561-lsn-0-200.snap
xid-561-lsn-0-300.snap
xid-561-lsn-0-400.snap
xid-561-lsn-0-500.snap
xid-561-lsn-0-600.snap
xid-561-lsn-0-700.snap
xid-561-lsn-0-800.snap
xid-561-lsn-0-900.snap

5. Kill publisher's postgres process before COMMIT.

  @pub $ kill -s SIGKILL [pid of postgres]

6. Start publisher's postgres process.

  @pub $ pg_ctl start -D ${PGDATA}

7. After a while, we can see the files remaining.
  (Immediately after starting publiser, we can not see these files.)

  @pub $ pg_ctl start -D ${PGDATA}

  When I configured with '--enable-cassert', below assertion error
  was appeared.

TRAP: FailedAssertion("!(txn->final_lsn != 0)", File:  
"reorderbuffer.c", Line: 2576)



Attached patch sets final_lsn to the last ReorderBufferChange if
final_lsn == 0.

--
Atsushi Torikoshi
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

diff --git a/src/backend/replication/logical/reorderbuffer.c 
b/src/backend/replication/logical/reorderbuffer.c
index 0f607ba..51d5b71 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1093,6 +1093,20 @@ ReorderBufferCleanupTXN(ReorderBuffer *rb, 
ReorderBufferTXN *txn)
ReorderBufferCleanupTXN(rb, subtxn);
}
 
+   /*
+* If this transaction encountered crash during transaction,
+* txn->final_lsn remains initial value.
+* To properly remove entries which were spilled to disk, we need valid
+* final_lsn.
+* So we set final_lsn to the lsn of the last ReorderBufferChange.
+*/
+   if (txn->final_lsn == 0)
+   {
+   ReorderBufferChange *last_change =
+   dlist_tail_element(ReorderBufferChange, node, >changes);
+   txn->final_lsn = last_change->lsn;
+   }
+
/* cleanup changes in the toplevel txn */
dlist_foreach_modify(iter, >changes)
{

Re: Jsonb transform for pl/python

[Anthony Bykov]

On Mon, 13 Nov 2017 15:08:16 +
Aleksander Alekseev  wrote:

> The following review has been posted through the commitfest
> application: make installcheck-world:  tested, passed
> Implements feature:   tested, passed
> Spec compliant:   tested, passed
> Documentation:tested, passed
> 
> Hi Anthony,
> 
> Thank you for the new version of the patch! Here is my code review.
> 
> 1. In jsonb_plpython2.out:
> 
> +CREATE FUNCTION back(val jsonb) RETURNS jsonb
> +LANGUAGE plpython2u
> +TRANSFORM FOR TYPE jsonb
> +as $$
> +return val
> +$$;
> +SELECT back('null'::jsonb);
> +  back  
> +
> + [null]
> +(1 row)
> +
> +SELECT back('1'::jsonb);
> + back 
> +--
> + [1]
> +(1 row)
> +
> +SELECT back('true'::jsonb);
> +  back  
> +
> + [true]
> +(1 row)
> 
> Maybe I'm missing something, but why exactly all JSONB values turn
> into arrays?
> 
> 2. Could you please also add tests for some negative and real
> numbers? Also could you check that your code handles numbers like
> MAX_INT, MIN_INT, +/- infinity and NaN properly in both (Python <->
> JSONB) directions?
> 
> 3. Handling unicode strings properly is another thing that is worth
> checking.
> 
> 4. I think we also need some tests that check the behavior of Python
> -> JSONB conversion when the object contains data that is not
> representable in JSON format, e.g. set(), some custom objects, etc. 
> 
> 5. PyObject_FromJsonbValue - I realize it's unlikely that the new
> jsonbValue->type will be introduced any time soon. Still I believe
> it's a good practice to add "it should never happen" default case
> that just does elog(ERROR, ...) in case it happens nevertheless.
> Otherwise in this scenario instead of reporting the error the code
> will silently do the wrong thing.
> 
> 6. Well, you decided to make the extension non-relocatable. Could you
> at least describe what prevents it to be relocatable or why it's
> meaningless is a comment in .control file? Please note that almost
> all contrib/ extensions are relocatable. I believe your extension
> should be relocatable as well unless there is a good reason why it
> can't.
> 
> The new status of this patch is: Waiting on Author

Hi,
thank you for your review. I took your comments into account in the
third version of the patch. In the new version, I've added all the
tests you asked for. The interesting thing is that:
1. set or any other non-jsonb-transformable object is transformed into
string and added to jsonb as a string. 
2. couldn't find a solution of working with "inf": this extension
troughs exception if python generates a number called "inf" and returns
it, but if we pass a very large integer into a function, it works fine
with the whole number. This situation can be seen in tests.

I've added tests of large numerics which weights quite heavy. So,
please find it in compressed format in attachments.


--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

0001-jsonb_plpython-extension-v3.patch.gz
Description: application/gzip