public inbox for [email protected]  
help / color / mirror / Atom feed
From: David Rowley <[email protected]>
To: Andy Fan <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Amit Langote <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: jian he <[email protected]>
Cc: Chapman Flack <[email protected]>
Cc: [email protected]
Subject: Re: Extract numeric filed in JSONB more effectively
Date: Thu, 12 Sep 2024 09:00:41 +1200
Message-ID: <CAApHDvqnUONCN54dAXdH678ELK7aH2gyVhh0GjTRtrexPd9YMw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAKU4AWoqAVya6PBhn+BCbFaBMt3z-2=i5fKO3bW=6HPhbid2Dw@mail.gmail.com>
	<CAKU4AWo4es1awVNbjLZkEpYCkgjxhkszbHiMF0UTsjx62K5RQg@mail.gmail.com>
	<169880504467.94392.3769687331705514588.pgcf@coridan.postgresql.org>
	<[email protected]>
	<CACJufxHRMDDehg9Py8+3Yh9JqTjZ=MF6pbxqvX_KANLpC+X1cA@mail.gmail.com>
	<[email protected]>
	<CACJufxH-6DkAGr0GRFSS7BAwKG7uVFceXtxHLM7Ub7oGagQGKg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Wed, 17 Apr 2024 at 17:17, Andy Fan <[email protected]> wrote:
> rebase to the latest master again.

There's a lot of complexity in the v18 patch that I don't understand
the need for.

I imagined you'd the patch should create a SupportRequestSimplify
support function for jsonb_numeric() that checks if the input
expression is an OpExpr with funcid of jsonb_object_field().  All you
do then is ditch the cast and change the OpExpr to call a new function
named jsonb_object_field_numeric() which returns the val.numeric
directly.  Likely the same support function could handle jsonb casts
to other types too, in which case you'd just call some other function,
e.g jsonb_object_field_timestamp() or jsonb_object_field_boolean().

Can you explain why the additional complexity is needed over what's in
the attached patch?

David

diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 928552d551..7b60b36189 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -18,7 +18,9 @@
 #include "funcapi.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
+#include "nodes/supportnodes.h"
 #include "utils/builtins.h"
+#include "utils/fmgroids.h"
 #include "utils/json.h"
 #include "utils/jsonb.h"
 #include "utils/jsonfuncs.h"
@@ -2069,6 +2071,64 @@ jsonb_numeric(PG_FUNCTION_ARGS)
 	PG_RETURN_NUMERIC(retValue);
 }
 
+Datum
+jsonb_object_field_numeric(PG_FUNCTION_ARGS)
+{
+	Jsonb *jb = PG_GETARG_JSONB_P(0);
+	text *key = PG_GETARG_TEXT_PP(1);
+	JsonbValue *v;
+	JsonbValue vbuf;
+
+	if (!JB_ROOT_IS_OBJECT(jb))
+		PG_RETURN_NULL();
+
+	v = getKeyJsonValueFromContainer(&jb->root,
+									 VARDATA_ANY(key),
+									 VARSIZE_ANY_EXHDR(key),
+									 &vbuf);
+
+	if (v != NULL)
+	{
+		if (v->type == jbvNumeric)
+			PG_RETURN_NUMERIC(v->val.numeric);
+
+		cannotCastJsonbValue(v->type, "numeric");
+	}
+
+	PG_RETURN_NULL();
+}
+
+Datum
+jsonb_numeric_support(PG_FUNCTION_ARGS)
+{
+	Node	   *rawreq = (Node *) PG_GETARG_POINTER(0);
+	Node *ret = NULL;
+
+	if (IsA(rawreq, SupportRequestSimplify))
+	{
+		SupportRequestSimplify *req = (SupportRequestSimplify *) rawreq;
+		FuncExpr *func = req->fcall;
+		OpExpr *opexpr = list_nth(func->args, 0);
+
+		/*
+		 * Transform jsonb_object_field calls that directly cast to numeric
+		 * into a direct call to jsonb_object_field_numeric.  This allows us
+		 * to directly access the numeric field and return it directly thus
+		 * saving casting from jsonb to numeric.
+		 */
+		if (IsA(opexpr, OpExpr) && opexpr->opfuncid == F_JSONB_OBJECT_FIELD)
+		{
+			opexpr->opfuncid = F_JSONB_OBJECT_FIELD_NUMERIC;
+			PG_RETURN_POINTER(opexpr);
+		}
+
+		PG_RETURN_POINTER(ret);
+	}
+
+	PG_RETURN_POINTER(ret);
+}
+
+
 Datum
 jsonb_int2(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index ff5436acac..10aed0b0b1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4642,8 +4642,12 @@
   proname => 'bool', prorettype => 'bool', proargtypes => 'jsonb',
   prosrc => 'jsonb_bool' },
 { oid => '3449', descr => 'convert jsonb to numeric',
-  proname => 'numeric', prorettype => 'numeric', proargtypes => 'jsonb',
+  proname => 'numeric', prosupport => 'jsonb_numeric_support',
+  prorettype => 'numeric', proargtypes => 'jsonb',
   prosrc => 'jsonb_numeric' },
+{ oid => '8394', descr => 'planner support for jsonb casting support',
+  proname => 'jsonb_numeric_support',  prorettype => 'internal',
+  proargtypes => 'internal', prosrc => 'jsonb_numeric_support' },
 { oid => '3450', descr => 'convert jsonb to int2',
   proname => 'int2', prorettype => 'int2', proargtypes => 'jsonb',
   prosrc => 'jsonb_int2' },
@@ -10083,6 +10087,10 @@
   proargtypes => 'jsonb _text', proallargtypes => '{jsonb,_text}',
   proargmodes => '{i,v}', proargnames => '{from_json,path_elems}',
   prosrc => 'jsonb_extract_path' },
+{ oid => '8395',
+  proname => 'jsonb_object_field_numeric', prorettype => 'numeric',
+  proargtypes => 'jsonb text', proargnames => '{from_json, field_name}',
+  prosrc => 'jsonb_object_field_numeric' },
 { oid => '3940', descr => 'get value from jsonb as text with path elements',
   proname => 'jsonb_extract_path_text', provariadic => 'text',
   prorettype => 'text', proargtypes => 'jsonb _text',


Attachments:

  [text/plain] jsonb_numeric_support.patch.txt (3.4K, 2-jsonb_numeric_support.patch.txt)
  download | inline diff:
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 928552d551..7b60b36189 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -18,7 +18,9 @@
 #include "funcapi.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
+#include "nodes/supportnodes.h"
 #include "utils/builtins.h"
+#include "utils/fmgroids.h"
 #include "utils/json.h"
 #include "utils/jsonb.h"
 #include "utils/jsonfuncs.h"
@@ -2069,6 +2071,64 @@ jsonb_numeric(PG_FUNCTION_ARGS)
 	PG_RETURN_NUMERIC(retValue);
 }
 
+Datum
+jsonb_object_field_numeric(PG_FUNCTION_ARGS)
+{
+	Jsonb *jb = PG_GETARG_JSONB_P(0);
+	text *key = PG_GETARG_TEXT_PP(1);
+	JsonbValue *v;
+	JsonbValue vbuf;
+
+	if (!JB_ROOT_IS_OBJECT(jb))
+		PG_RETURN_NULL();
+
+	v = getKeyJsonValueFromContainer(&jb->root,
+									 VARDATA_ANY(key),
+									 VARSIZE_ANY_EXHDR(key),
+									 &vbuf);
+
+	if (v != NULL)
+	{
+		if (v->type == jbvNumeric)
+			PG_RETURN_NUMERIC(v->val.numeric);
+
+		cannotCastJsonbValue(v->type, "numeric");
+	}
+
+	PG_RETURN_NULL();
+}
+
+Datum
+jsonb_numeric_support(PG_FUNCTION_ARGS)
+{
+	Node	   *rawreq = (Node *) PG_GETARG_POINTER(0);
+	Node *ret = NULL;
+
+	if (IsA(rawreq, SupportRequestSimplify))
+	{
+		SupportRequestSimplify *req = (SupportRequestSimplify *) rawreq;
+		FuncExpr *func = req->fcall;
+		OpExpr *opexpr = list_nth(func->args, 0);
+
+		/*
+		 * Transform jsonb_object_field calls that directly cast to numeric
+		 * into a direct call to jsonb_object_field_numeric.  This allows us
+		 * to directly access the numeric field and return it directly thus
+		 * saving casting from jsonb to numeric.
+		 */
+		if (IsA(opexpr, OpExpr) && opexpr->opfuncid == F_JSONB_OBJECT_FIELD)
+		{
+			opexpr->opfuncid = F_JSONB_OBJECT_FIELD_NUMERIC;
+			PG_RETURN_POINTER(opexpr);
+		}
+
+		PG_RETURN_POINTER(ret);
+	}
+
+	PG_RETURN_POINTER(ret);
+}
+
+
 Datum
 jsonb_int2(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index ff5436acac..10aed0b0b1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4642,8 +4642,12 @@
   proname => 'bool', prorettype => 'bool', proargtypes => 'jsonb',
   prosrc => 'jsonb_bool' },
 { oid => '3449', descr => 'convert jsonb to numeric',
-  proname => 'numeric', prorettype => 'numeric', proargtypes => 'jsonb',
+  proname => 'numeric', prosupport => 'jsonb_numeric_support',
+  prorettype => 'numeric', proargtypes => 'jsonb',
   prosrc => 'jsonb_numeric' },
+{ oid => '8394', descr => 'planner support for jsonb casting support',
+  proname => 'jsonb_numeric_support',  prorettype => 'internal',
+  proargtypes => 'internal', prosrc => 'jsonb_numeric_support' },
 { oid => '3450', descr => 'convert jsonb to int2',
   proname => 'int2', prorettype => 'int2', proargtypes => 'jsonb',
   prosrc => 'jsonb_int2' },
@@ -10083,6 +10087,10 @@
   proargtypes => 'jsonb _text', proallargtypes => '{jsonb,_text}',
   proargmodes => '{i,v}', proargnames => '{from_json,path_elems}',
   prosrc => 'jsonb_extract_path' },
+{ oid => '8395',
+  proname => 'jsonb_object_field_numeric', prorettype => 'numeric',
+  proargtypes => 'jsonb text', proargnames => '{from_json, field_name}',
+  prosrc => 'jsonb_object_field_numeric' },
 { oid => '3940', descr => 'get value from jsonb as text with path elements',
   proname => 'jsonb_extract_path_text', provariadic => 'text',
   prorettype => 'text', proargtypes => 'jsonb _text',


view thread (28+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Extract numeric filed in JSONB more effectively
  In-Reply-To: <CAApHDvqnUONCN54dAXdH678ELK7aH2gyVhh0GjTRtrexPd9YMw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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