pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Implementation of subscripting for jsonb
5+ messages / 4 participants
[nested] [flat]

* pgsql: Implementation of subscripting for jsonb
@ 2021-01-31 20:54  Alexander Korotkov <akorotkov@postgresql.org>
  0 siblings, 1 reply; 5+ messages in thread

From: Alexander Korotkov @ 2021-01-31 20:54 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Implementation of subscripting for jsonb

Subscripting for jsonb does not support slices, does not have a limit for the
number of subscripts, and an assignment expects a replace value to have jsonb
type.  There is also one functional difference between assignment via
subscripting and assignment via jsonb_set().  When an original jsonb container
is NULL, the subscripting replaces it with an empty jsonb and proceeds with
an assignment.

For the sake of code reuse, we rearrange some parts of jsonb functionality
to allow the usage of the same functions for jsonb_set and assign subscripting
operation.

The original idea belongs to Oleg Bartunov.

Catversion is bumped.

Discussion: https://postgr.es/m/CA%2Bq6zcV8qvGcDXurwwgUbwACV86Th7G80pnubg42e-p9gsSf%3Dg%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2Bq6zcX3mdxGCgdThzuySwH-ApyHHM-G4oB1R0fn0j2hZqqkLQ%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2Bq6zcVDuGBv%3DM0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2Bq6zcVovR%2BXY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA%40mail.gmail.com
Author: Dmitry Dolgov
Reviewed-by: Tom Lane, Arthur Zakirov, Pavel Stehule, Dian M Fay
Reviewed-by: Andrew Dunstan, Chapman Flack, Merlin Moncure, Peter Geoghegan
Reviewed-by: Alvaro Herrera, Jim Nasby, Josh Berkus, Victor Wagner
Reviewed-by: Aleksander Alekseev, Robert Haas, Oleg Bartunov

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/676887a3b0b8e3c0348ac3f82ab0d16e9a24bd43

Modified Files
--------------
doc/src/sgml/json.sgml              |  51 +++++
src/backend/utils/adt/Makefile      |   1 +
src/backend/utils/adt/jsonb_util.c  |  72 ++++++-
src/backend/utils/adt/jsonbsubs.c   | 412 ++++++++++++++++++++++++++++++++++++
src/backend/utils/adt/jsonfuncs.c   | 188 ++++++++--------
src/include/catalog/catversion.h    |   2 +-
src/include/catalog/pg_proc.dat     |   4 +
src/include/catalog/pg_type.dat     |   3 +-
src/include/utils/jsonb.h           |   6 +-
src/test/regress/expected/jsonb.out | 272 +++++++++++++++++++++++-
src/test/regress/sql/jsonb.sql      |  84 +++++++-
src/tools/pgindent/typedefs.list    |   1 +
12 files changed, 988 insertions(+), 108 deletions(-)



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: pgsql: Implementation of subscripting for jsonb
@ 2021-02-01 06:55  Heikki Linnakangas <hlinnaka@iki.fi>
  parent: Alexander Korotkov <akorotkov@postgresql.org>
  0 siblings, 1 reply; 5+ messages in thread

From: Heikki Linnakangas @ 2021-02-01 06:55 UTC (permalink / raw)
  To: Alexander Korotkov <akorotkov@postgresql.org>; +Cc: pgsql-committers@lists.postgresql.org

On 31/01/2021 22:54, Alexander Korotkov wrote:
> Implementation of subscripting for jsonb

The Itanium and sparc64 buildfarm members didn't like this, and are 
crashing at "select ('123'::jsonb)['a'];". Unaligned memory access, perhaps?

- Heikki





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: pgsql: Implementation of subscripting for jsonb
@ 2021-02-01 07:05  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 1 reply; 5+ messages in thread

From: Tom Lane @ 2021-02-01 07:05 UTC (permalink / raw)
  To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org

Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 31/01/2021 22:54, Alexander Korotkov wrote:
>> Implementation of subscripting for jsonb

> The Itanium and sparc64 buildfarm members didn't like this, and are 
> crashing at "select ('123'::jsonb)['a'];". Unaligned memory access, perhaps?

I think I just identified the cause.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: pgsql: Implementation of subscripting for jsonb
@ 2021-02-01 12:41  Alexander Korotkov <aekorotkov@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 5+ messages in thread

From: Alexander Korotkov @ 2021-02-01 12:41 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>

On Mon, Feb 1, 2021 at 10:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
> > On 31/01/2021 22:54, Alexander Korotkov wrote:
> >> Implementation of subscripting for jsonb
>
> > The Itanium and sparc64 buildfarm members didn't like this, and are
> > crashing at "select ('123'::jsonb)['a'];". Unaligned memory access, perhaps?
>
> I think I just identified the cause.

Thanks again for fixing this.

BTW, I managed to reproduce the issue by compiling with CFLAGS="-O0
-fsanitize=alignment -fsanitize-trap=alignment" and the patch
attached.

I can propose the following to catch such issues earlier.  We could
finish (wrap attribute with macro and apply it to other places with
misalignment access if any) and apply the attached patch and make
commitfest.cputube.org check patches with CFLAGS="-O0
-fsanitize=alignment -fsanitize-trap=alignment".  What do you think?

------
Regards,
Alexander Korotkov

Attachments:

pg_comp_crc32c_sse42_no_sanitize_alignment.patch (application/octet-stream, 348B) download diff --git a/src/port/pg_crc32c_sse42.c b/src/port/pg_crc32c_sse42.c index 3b94a7388ab..cc5f8f9b487 100644 --- a/src/port/pg_crc32c_sse42.c +++ b/src/port/pg_crc32c_sse42.c @@ -18,6 +18,7 @@ #include "port/pg_crc32c.h" +__attribute__((no_sanitize("alignment"))) pg_crc32c pg_comp_crc32c_sse42(pg_crc32c crc, const void *data, size_t len) {
^ permalink raw reply [nested|flat] 5+ messages in thread

* Re: pgsql: Implementation of subscripting for jsonb
@ 2021-02-01 13:00  Alexander Korotkov <aekorotkov@gmail.com>
  parent: Alexander Korotkov <aekorotkov@gmail.com>
  0 siblings, 0 replies; 5+ messages in thread

From: Alexander Korotkov @ 2021-02-01 13:00 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>

On Mon, Feb 1, 2021 at 3:41 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Mon, Feb 1, 2021 at 10:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Heikki Linnakangas <hlinnaka@iki.fi> writes:
> > > On 31/01/2021 22:54, Alexander Korotkov wrote:
> > >> Implementation of subscripting for jsonb
> >
> > > The Itanium and sparc64 buildfarm members didn't like this, and are
> > > crashing at "select ('123'::jsonb)['a'];". Unaligned memory access, perhaps?
> >
> > I think I just identified the cause.
>
> Thanks again for fixing this.
>
> BTW, I managed to reproduce the issue by compiling with CFLAGS="-O0
> -fsanitize=alignment -fsanitize-trap=alignment" and the patch
> attached.
>
> I can propose the following to catch such issues earlier.  We could
> finish (wrap attribute with macro and apply it to other places with
> misalignment access if any) and apply the attached patch and make
> commitfest.cputube.org check patches with CFLAGS="-O0
> -fsanitize=alignment -fsanitize-trap=alignment".  What do you think?

The revised patch is attached.  The attribute is wrapped into
pg_attribute_no_sanitize_alignment() macro.  I've checked it works for
me with gcc-10 and clang-11.

------
Regards,
Alexander Korotkov

Attachments:

pg_attribute_no_sanitize_alignment.patch (application/octet-stream, 904B) download diff --git a/src/include/c.h b/src/include/c.h index ae978830daf..47b97a9eb73 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -120,6 +120,13 @@ #define pg_attribute_unused() #endif +/* Macro for disabling GCC alignment sanitizer */ +#ifdef __GNUC__ +#define pg_attribute_no_sanitize_alignment() __attribute__((no_sanitize("alignment"))) +#else +#define pg_attribute_no_sanitize_alignment() +#endif + /* * pg_nodiscard means the compiler should warn if the result of a function * call is ignored. The name "nodiscard" is chosen in alignment with diff --git a/src/port/pg_crc32c_sse42.c b/src/port/pg_crc32c_sse42.c index 3b94a7388ab..10fc01e1f06 100644 --- a/src/port/pg_crc32c_sse42.c +++ b/src/port/pg_crc32c_sse42.c @@ -18,6 +18,7 @@ #include "port/pg_crc32c.h" +pg_attribute_no_sanitize_alignment() pg_crc32c pg_comp_crc32c_sse42(pg_crc32c crc, const void *data, size_t len) {
^ permalink raw reply [nested|flat] 5+ messages in thread


end of thread, other threads:[~2021-02-01 13:00 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-31 20:54 pgsql: Implementation of subscripting for jsonb Alexander Korotkov <akorotkov@postgresql.org>
2021-02-01 06:55 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-02-01 07:05   ` Tom Lane <tgl@sss.pgh.pa.us>
2021-02-01 12:41     ` Alexander Korotkov <aekorotkov@gmail.com>
2021-02-01 13:00       ` Alexander Korotkov <aekorotkov@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox