> On Jan 27, 2020, at 5:30 AM, Robert Haas <robertmh...@gmail.com> wrote:
> 
> On Sun, Jan 26, 2020 at 9:05 PM Mark Dilger
> <mark.dil...@enterprisedb.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: v6-0001-Relocating-jsonapi-to-common.patch
Description: Binary data

Attachment: v6-0002-Fixed-warning-for-json_errdetail.patch
Description: Binary data

Attachment: v6-0003-Adding-frontend-tests-for-json-parser.patch
Description: Binary data



Reply via email to