Dependency to logging in jsonapi.c

[Michael Paquier]

Hi all,

jsonapi.c includes the following code bits to enforce the use of
logging:
#ifdef FRONTEND
#define check_stack_depth()
#define json_log_and_abort(...) \
   do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
#else
#define json_log_and_abort(...) elog(ERROR, __VA_ARGS__)
#endif

This has been mentioned here:
https://www.postgresql.org/message-id/ynfxpfebvfu2h...@paquier.xyz

This requires any tools in the frontend to use pg_logging_init(),
which is recommended, but not enforced.  Perhaps that's fine in
itself to require frontends to register to the central logging APIs,
but json_log_and_abort() gets only called when dealing with incorrect
error codes even if we rely on JsonParseErrorType in all the places
doing error handling with the JSON parsing.  And requiring a
dependency on logging just for unlikely-to-happen cases seems a bit
crazy to me.

Attached is a suggestion of patch to rework that a bit.  Some extra
elog()s could be added for the backend, as well as a new error code to
use as default of report_parse_error(), but that does not seem to gain
much.  And this item looks independent of switching this code to use
pqexpbuffer.h to be more portable with issues like OOM problems.

Thoughts?
--
Michael

diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index d376ab152d..624b300994 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -20,20 +20,10 @@
 #include "common/jsonapi.h"
 #include "mb/pg_wchar.h"
 
-#ifdef FRONTEND
-#include "common/logging.h"
-#else
+#ifndef FRONTEND
 #include "miscadmin.h"
 #endif
 
-#ifdef FRONTEND
-#define check_stack_depth()
-#define json_log_and_abort(...) \
-	do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
-#else
-#define json_log_and_abort(...) elog(ERROR, __VA_ARGS__)
-#endif
-
 /*
  * The context of the parser is maintained by the recursive descent
  * mechanism, but is passed explicitly to the error reporting routine
@@ -378,7 +368,9 @@ parse_object(JsonLexContext *lex, JsonSemAction *sem)
 	JsonTokenType tok;
 	JsonParseErrorType result;
 
+#ifndef FRONTEND
 	check_stack_depth();
+#endif
 
 	if (ostart != NULL)
 		(*ostart) (sem->semstate);
@@ -478,7 +470,9 @@ parse_array(JsonLexContext *lex, JsonSemAction *sem)
 	json_struct_action aend = sem->array_end;
 	JsonParseErrorType result;
 
+#ifndef FRONTEND
 	check_stack_depth();
+#endif
 
 	if (astart != NULL)
 		(*astart) (sem->semstate);
@@ -1047,7 +1041,6 @@ report_parse_error(JsonParseContext ctx, JsonLexContext *lex)
 	 * unhandled enum values.  But this needs to be here anyway to cover the
 	 * possibility of an incorrect input.
 	 */
-	json_log_and_abort("unexpected json parse state: %d", (int) ctx);
 	return JSON_SUCCESS;		/* silence stupider compilers */
 }
 
@@ -1115,8 +1108,7 @@ json_errdetail(JsonParseErrorType error, JsonLexContext *lex)
 	 * unhandled enum values.  But this needs to be here anyway to cover the
 	 * possibility of an incorrect input.
 	 */
-	json_log_and_abort("unexpected json parse error type: %d", (int) error);
-	return NULL;				/* silence stupider compilers */
+	return psprintf("unexpected json parse error type: %d", (int) error);
 }
 
 /*

signature.asc
Description: PGP signature