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

Thanks for your review.  I considered all of this along with your review comments in another email prior to sending v7 in response to that other email a few minutes ago.

> On Jan 28, 2020, at 7:17 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> 
> On Mon, Jan 27, 2020 at 3:05 PM Mark Dilger
> <mark.dilger@enterprisedb.com> wrote:
>> 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.
> 
> OK, so I think this is getting close.
> 
> What is now 0001 manages to have four (4) conditionals on FRONTEND at
> the top of the file. This seems like at least one two many. 

You are referencing this section, copied here from the patch:

> #ifndef FRONTEND
> #include "postgres.h"
> #else
> #include "postgres_fe.h"
> #endif
> 
> #include "common/jsonapi.h"
> 
> #ifdef FRONTEND
> #include "common/logging.h"
> #endif
> 
> #include "mb/pg_wchar.h"
> 
> #ifndef FRONTEND
> #include "miscadmin.h"
> #endif

I merged these a bit.  See v7-0001 for details.

> Also, the preprocessor police are on their way to your house now to
> arrest you for that first one. You need to write it like this:
> 
> #define json_log_and_abort(...) \
>    do { pg_log_fatal(__VA_ARGS__); exit(1); } while (0)

Yes, right, I had done that and somehow didn’t get it into the patch.  I’ll have coffee and donuts waiting.

> {
> - JsonLexContext *lex = palloc0(sizeof(JsonLexContext));
> + JsonLexContext *lex;
> +
> +#ifndef FRONTEND
> + lex = palloc0(sizeof(JsonLexContext));
> +#else
> + lex = (JsonLexContext*) malloc(sizeof(JsonLexContext));
> + memset(lex, 0, sizeof(JsonLexContext));
> +#endif
> 
> Instead of this, how making no change at all here?

Yes, good point.  I had split that into frontend vs backend because I was using palloc0fast for the backend, which seems to me the preferred function when the size is compile-time known, like it is here, and there is no palloc0fast in fe_memutils.h for frontend use.  I then changed back to palloc0 when I noticed that pretty much nowhere else similar to this in the project uses palloc0fast.  I neglected to change back completely, which left what you are quoting.

Out of curiousity, why is palloc0fast not used in more places?

> - default:
> - elog(ERROR, "unexpected json parse state: %d", ctx);
>  }
> +
> + /* Not reached */
> + json_log_and_abort("unexpected json parse state: %d", ctx);
> 
> This, too, seems unnecessary.

This was in response to Mahendra’s report of a compiler warning, which I didn’t get on my platform. The code in master changed a bit since v6 was written, so v7 just goes with how the newer committed code does this.

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