Re: Transform for pl/perl

I wrote: > The remaining unresolved issue in this thread is Ilmari's suggestion > that we should convert integers to Perl IV (or UV?) if they fit, rather > than always convert to NV as now. Oh ... after re-reading the thread I realized there was one other point that we'd all forgotten about,

Re: Transform for pl/perl

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > [ 0001-Fix-excess-enreferencing-in-plperl-jsonb-transform.patch ] I tested this a bit more thoroughly by dint of applying Data::Dumper to the Perl values, and found that we were still getting extra references to sub-objects,

Re: Transform for pl/perl

On 2018-Jun-08, Tom Lane wrote: > I wrote: > > I'm inclined to think that auto-dereference is indeed a good idea, > > and am tempted to go make that change to make all this consistent. > > Comments? > > Here's a draft patch for that. I ended up only changing > plperl_sv_to_datum. There is

Re: Transform for pl/perl

I wrote: > I'm inclined to think that auto-dereference is indeed a good idea, > and am tempted to go make that change to make all this consistent. > Comments? Here's a draft patch for that. I ended up only changing plperl_sv_to_datum. There is maybe a case for doing something similar in

Re: Transform for pl/perl

I wrote: > ... So if we think that \undef ought to > produce a SQL null, the thing to do is move the dereferencing loop to > the beginning of plperl_sv_to_datum, and then \undef would produce NULL > whether this transform is installed or not. I don't have a well-informed > opinion on whether

Re: Transform for pl/perl

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > Peter Eisentraut writes: >> The way I understand it, it's only how things are passed around >> internally. Nothing is noticeable externally, and so there is no >> backward compatibility issue. >> >> At least that's how I

Re: Transform for pl/perl

Peter Eisentraut writes: > On 6/6/18 12:14, Alvaro Herrera wrote: >> On 2018-May-17, Peter Eisentraut wrote: >> >>> The items that are still open from the original email are: >>> >>> 2) jsonb scalar values are passed to the plperl function wrapped in not >>>one, but _two_ layers of

Re: Transform for pl/perl

On 6/6/18 12:14, Alvaro Herrera wrote: > On 2018-May-17, Peter Eisentraut wrote: > >> The items that are still open from the original email are: >> >> 2) jsonb scalar values are passed to the plperl function wrapped in not >>one, but _two_ layers of references >> >> 3) jsonb numeric values

Re: Transform for pl/perl

On 2018-May-17, Peter Eisentraut wrote: > The items that are still open from the original email are: > > 2) jsonb scalar values are passed to the plperl function wrapped in not >one, but _two_ layers of references > > 3) jsonb numeric values are passed as perl's NV (floating point) type, >

Re: Transform for pl/perl

; this will result in other strange behaviors. > > Nubers > 2⁵³ are not "interoperable" in the sense of the JSON spec, > because JavaScript only has doubles, but it seems desirable to > preserve whatever precision one reasonably can, and I can't think of > any downsides. We alr

Re: Transform for pl/perl

On 5/17/18 17:11, Alvaro Herrera wrote: > This is still listed as an open item, though the patch proposed by Peter > upthread has been committed. If I understand correctly, ilmari was > going to propose another patch. Or is the right course of action to set > the open item as resolved? The

Re: Transform for pl/perl

Alvaro Herrera writes: > This is still listed as an open item, though the patch proposed by Peter > upthread has been committed. If I understand correctly, ilmari was > going to propose another patch. Or is the right course of action to set > the open item as resolved?

Re: Transform for pl/perl

ON spec, > because JavaScript only has doubles, but it seems desirable to preserve > whatever precision one reasonably can, and I can't think of any > downsides. We already support the full numeric range when processing > JSONB in SQL, it's just in the PL/Perl transform (and pos

Re: Transform for pl/perl

s. We already support the full numeric range when processing JSONB in SQL, it's just in the PL/Perl transform (and possibly PL/Python, I didn't look) we're losing precision. Perl can also be configured to use long double or __float128 (via libquadmath) for its NV type, but I think preserving 64bit in

Re: Transform for pl/perl

These two items are now outstanding: On 4/10/18 07:33, Dagfinn Ilmari Mannsåker wrote: > 2) jsonb scalar values are passed to the plperl function wrapped in not >one, but _two_ layers of references I don't understand this one, or why it's a problem, or what to do about it. > 3) jsonb

Re: Transform for pl/perl

On 4/29/18 14:28, Tom Lane wrote: > Peter Eisentraut writes: >> On 4/24/18 14:31, Andrew Dunstan wrote: >>> There is the routine IsValidJsonNumber that helps - see among others >>> hstore_io.c for an example use. > >> I would need something like that taking a

Re: Transform for pl/perl

Peter Eisentraut writes: > On 4/24/18 14:31, Andrew Dunstan wrote: >> There is the routine IsValidJsonNumber that helps - see among others >> hstore_io.c for an example use. > I would need something like that taking a double/float8 input. But I > think there is

Re: Transform for pl/perl

On 4/24/18 14:31, Andrew Dunstan wrote: > There is the routine IsValidJsonNumber that helps - see among others > hstore_io.c for an example use. I would need something like that taking a double/float8 input. But I think there is no such shortcut available, so I just wrote out the tests for isinf

Re: Transform for pl/perl

On 04/24/2018 12:17 PM, Peter Eisentraut wrote: > On 4/10/18 10:31, Dagfinn Ilmari Mannsåker wrote: >> Also, it doesn't parse back in as jsonb either: >> >> =# select jsonbnan()::text::json; >> ERROR: invalid input syntax for type json >> DETAIL: Token "NaN" is invalid. >>

Re: Transform for pl/perl

On 4/10/18 10:31, Dagfinn Ilmari Mannsåker wrote: > Also, it doesn't parse back in as jsonb either: > > =# select jsonbnan()::text::json; > ERROR: invalid input syntax for type json > DETAIL: Token "NaN" is invalid. > CONTEXT: JSON data, line 1: NaN > > And it's inconsistent

Re: Transform for pl/perl

On 4/10/18 07:33, Dagfinn Ilmari Mannsåker wrote: > 1) both the jsonb_plperl and jsonb_plperlu extensions contain the SQL >functions jsonb_to_plperl and plperl_to_jsonb, so can't be installed >simultaneously > > 2) jsonb scalar values are passed to the plperl function wrapped in not >

Re: Transform for pl/perl

ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > Tom Lane writes: > >> ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >>> While playing around some more with the extension, I discoverered a few >>> more issues: >>> ... >>> 4) SV_to_JsonbValue()

Re: Transform for pl/perl

Tom Lane writes: > ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >> While playing around some more with the extension, I discoverered a few >> more issues: >> ... >> 4) SV_to_JsonbValue() throws an error for infinite NVs, but not NaNs > > The others

Re: Transform for pl/perl

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > While playing around some more with the extension, I discoverered a few > more issues: > ... > 4) SV_to_JsonbValue() throws an error for infinite NVs, but not NaNs The others sound like bugs, but that one's intentional, since

Re: Transform for pl/perl

Tom Lane writes: > ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >> Tom Lane writes: >>> I think you'd have to convert to text and back. That's kind of icky, >>> but it beats failing. > >> I had a look, and that's what the

Re: Transform for pl/perl

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > Tom Lane writes: >> I think you'd have to convert to text and back. That's kind of icky, >> but it beats failing. > I had a look, and that's what the PL/Python transform does. Attached is > a patch that

Re: Transform for pl/perl

where it is, but you generally don't want to find yourself in the same spot." - Tollef Fog Heen >From acf968b4df81797fc06868dac87123413f3f4167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org> Date: Thu, 5 Apr 2018 16:23:59 +0100 S

Re: Transform for pl/perl

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > I tried fixing this by adding an 'if (SvUV(in))' clause to > SV_to_JsonbValue, but I couldn't find a function to create a numeric > value from an uint64. If it's not possible, should we error on UVs > greater than

Re: Transform for pl/perl

Peter Eisentraut writes: > On 3/15/18 03:46, Pavel Stehule wrote: >> It looks well >> >> the patch has tests and doc, >> there are not any warnings or compilation issues >> all tests passed >> >> I'll mark this patch as ready for commiter > > committed I

Re: Transform for pl/perl

On 3/15/18 03:46, Pavel Stehule wrote: > It looks well > > the patch has tests and doc, > there are not any warnings or compilation issues > all tests passed > > I'll mark this patch as ready for commiter committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL

Re: Transform for pl/perl

: >> > jsonb_plperl.c:142:9: warning: ‘result’ may be used uninitialized in >> > this function [-Wmaybe-uninitialized] >> > return result; >> > ^~ >> >> Hello, thanks for reviewing my patch! I really appreciate it. >> >> That warnin

Re: Transform for pl/perl

eciate it. That warnings are created on purpose - I was trying to prevent the case when new types are added into pl/perl, but new transform logic was not. Maybe there is a better way to do it, but right now I'll just add the "default: pg_unreachable" logic.

Re: Transform for pl/perl

hanks for reviewing my patch! I really appreciate it. That warnings are created on purpose - I was trying to prevent the case when new types are added into pl/perl, but new transform logic was not. Maybe there is a better way to do it, but right now I'll just add the "default: pg_un

Re: Transform for pl/perl

Hi I am looking on this patch. I found few issues: 1. compile warning I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -I/usr/lib64/perl5/CORE -c -o jsonb_plperl.o jsonb_plperl.c jsonb_plperl.c: In function ‘SV_FromJsonbValue’: jsonb_plperl.c:69:9: warning: ‘result’ may be used

Re: Transform for pl/perl

On Wed, 31 Jan 2018 13:36:22 +0300 Arthur Zakirov wrote: > I've noticed a possible bug: > > > + /* json key in v */ > > + key = > > pstrdup(v.val.string.val); > > +

Re: Transform for pl/perl

Hello, On Fri, Jan 12, 2018 at 11:47:39AM +0300, Anthony Bykov wrote: > Hello, thank you for your message. > The problem was that different perl compilers uses different infinity > representations. Some of them use "Inf" others - use "inf". So, in > attachments there is a new version of the

Re: Transform for pl/perl

On Fri, Jan 12, 2018 at 9:47 PM, Anthony Bykov wrote: > On Fri, 12 Jan 2018 15:19:26 +1300 > Thomas Munro wrote: > Hello, thank you for your message. > The problem was that different perl compilers uses different infinity > representations.

Re: Transform for pl/perl

On 01/12/2018 03:47 AM, Anthony Bykov wrote: > > The problem was that different perl compilers uses different infinity > representations. Some of them use "Inf" others - use "inf". So, in > attachments there is a new version of the patch. > There's a bit of an impedance mismatch and

Re: Transform for pl/perl

On Fri, 12 Jan 2018 15:19:26 +1300 Thomas Munro wrote: > On Thu, Dec 7, 2017 at 10:56 PM, Anthony Bykov > wrote: > >> Please, find a new version of the patch in attachments to this > >> message. > > Hi again Anthony, > > I wonder why

Re: Transform for pl/perl

On Thu, Dec 7, 2017 at 10:56 PM, Anthony Bykov wrote: >> Please, find a new version of the patch in attachments to this >> message. Hi again Anthony, I wonder why make check passes for me on my Mac, but when Travis CI (Ubuntu Trusty on amd64) runs it, it fails like this:

Re: Transform for pl/perl

On 12/1/17 13:15, Tom Lane wrote: > Robert Haas writes: >> On Fri, Dec 1, 2017 at 12:30 AM, Michael Paquier >> wrote: >>> Patch moved to CF 2018-01. Perhaps a committer will look at it at some >>> point. > >> FWIW, although I like the idea, I'm

Re: Transform for pl/perl

On Thu, 7 Dec 2017 12:54:55 +0300 Anthony Bykov wrote: > On Fri, 1 Dec 2017 15:49:21 -0300 > Alvaro Herrera wrote: > > > A few very minor comments while quickly paging through: > > > > 1. non-ASCII tests are going to cause problems in one

Re: Transform for pl/perl

On Fri, 1 Dec 2017 15:49:21 -0300 Alvaro Herrera wrote: > A few very minor comments while quickly paging through: > > 1. non-ASCII tests are going to cause problems in one platform or > another. Please don't include that one. > > 2. error messages >a) please use

Re: Transform for pl/perl

A few very minor comments while quickly paging through: 1. non-ASCII tests are going to cause problems in one platform or another. Please don't include that one. 2. error messages a) please use ereport() not elog() b) conform to style guidelines: errmsg() start with lowercase, others

Re: Transform for pl/perl

Robert Haas writes: > On Fri, Dec 1, 2017 at 12:30 AM, Michael Paquier > wrote: >> Patch moved to CF 2018-01. Perhaps a committer will look at it at some point. > FWIW, although I like the idea, I'm not going to work on the patch. I > like

Re: Transform for pl/perl

On Fri, Dec 1, 2017 at 12:30 AM, Michael Paquier wrote: > On Tue, Nov 28, 2017 at 5:14 PM, Aleksander Alekseev > wrote: >> The new status of this patch is: Ready for Committer > > Patch moved to CF 2018-01. Perhaps a committer will look at it

Re: Transform for pl/perl

On Tue, Nov 28, 2017 at 5:14 PM, Aleksander Alekseev wrote: > The new status of this patch is: Ready for Committer Patch moved to CF 2018-01. Perhaps a committer will look at it at some point. -- Michael

Re: Transform for pl/perl

On Wed, 15 Nov 2017 08:58:54 + 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 >

Re: [HACKERS] Transform for pl/perl

Hello, hackers. > > I'm curious, how much benefit we could get from this ? There are several > > publicly available json datasets, which can be used to measure performance > > gaining. I have bookmarks and review datasets available from > > http://www.sai.msu.su/~megera/postgres/files/, look at

Re: [HACKERS] Transform for pl/perl

2017-11-15 10:24 GMT+01:00 Oleg Bartunov : > > > On 14 Nov 2017 11:35, "Anthony Bykov" wrote: > > On Fri, 10 Nov 2017 14:40:21 +0100 > Pavel Stehule wrote: > > > Hi > > > > 2017-10-24 14:27 GMT+02:00 Anthony Bykov

Re: [HACKERS] Transform for pl/perl

On 14 Nov 2017 11:35, "Anthony Bykov" wrote: On Fri, 10 Nov 2017 14:40:21 +0100 Pavel Stehule wrote: > Hi > > 2017-10-24 14:27 GMT+02:00 Anthony Bykov : > > > There are some moments I should mention: > > 1. {"1":1}::jsonb

Re: [HACKERS] Transform for pl/perl

Hi > Hello, > Thank you for your review. I have fixed most of your comments, except > for the 5-th part, about data::dumper - I just couldn't understand > your point, but I've added more tests with more complex objects if this > helps. > > Please, take a look at new patch. You can find it in

Re: [HACKERS] Transform for pl/perl

On Fri, 10 Nov 2017 14:40:21 +0100 Pavel Stehule wrote: > Hi > > 2017-10-24 14:27 GMT+02:00 Anthony Bykov : > > > There are some moments I should mention: > > 1. {"1":1}::jsonb is transformed into HV {"1"=>"1"}, while > > ["1","2"]::jsonb is