Re: making the backend's json parser work in frontend code

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: making the backend's json parser work in frontend code
Date: 2020-01-27 20:05:44
Message-ID: CA41881F-6DDF-4E7C-8EA2-A01925913BB7@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jan 27, 2020, at 5:30 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Sun, Jan 26, 2020 at 9:05 PM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>> Ok, the tests pass. Here are those two patches again, both regenerated with a fresh invocation of ‘git format-patch’.
>
> Regarding 0006:
>
> +#ifndef FRONTEND
> #include "miscadmin.h"
> -#include "utils/jsonapi.h"
> +#endif
>
> I suggest
>
> #ifdef FRONTEND
> #define check_stack_depth()
> #else
> #include "miscadmin.h"
> #endif

Sure, we can do it that way.

> - lex->token_terminator = s + pg_mblen(s);
> + lex->token_terminator = s +
> pg_wchar_table[lex->input_encoding].mblen((const unsigned char *) s);
>
> Can we use pq_encoding_mblen() here? Regardless, it doesn't seem great
> to add more direct references to pg_wchar_table. I think we should
> avoid that.

Yes, that looks a lot cleaner.

>
> + return JSON_BAD_PARSER_STATE;
>
> I don't like this, either. I'm thinking about adding some
> variable-argument macros that either elog() in backend code or else
> pg_log_fatal() and exit(1) in frontend code. There are some existing
> precedents already (e.g. rmtree.c, pgfnames.c) which could perhaps be
> generalized. I think I'll start a new thread about that.

Right, you started the "pg_croak, or something like it?” thread, which already looks like it might not be resolved quickly. Can we use the

#ifndef FRONTEND
#define pg_log_warning(...) elog(WARNING, __VA_ARGS__)
#else
#include "common/logging.h"
#endif

pattern here as a place holder, and revisit it along with the other couple instances of this pattern if/when the “pg_croak, or something like it?” thread is ready for commit? I’m calling it json_log_and_abort(…) for now, as I can’t hope to guess what the final name will be.

I’m attaching a new patch set with these three changes including Mahendra’s patch posted elsewhere on this thread.

Since you’ve committed your 0004 and 0005 patches, this v6 patch set is now based on a fresh copy of master.


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
v6-0001-Relocating-jsonapi-to-common.patch application/octet-stream 11.0 KB
v6-0002-Fixed-warning-for-json_errdetail.patch application/octet-stream 848 bytes
v6-0003-Adding-frontend-tests-for-json-parser.patch application/octet-stream 9.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-01-27 20:34:19 Re: standby apply lag on inactive servers
Previous Message Pavel Stehule 2020-01-27 19:44:13 Re: [Proposal] Global temporary tables