Feedback-ID: ia2694551:Fastmail
Date: Thu, 18 Jan 2024 12:46:24 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Erik Rijkers <er@xs4all.nl>,
	jian he <jian.universality@gmail.com>, Andrew Dunstan <andrew@dunslane.net>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: remaining sql/json patches
Message-ID: <202401181146.fuoeskfzriq7@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+HiwqFTBYa+BipE3K=4--OC9f0-bK_n8Kh78Ez4fwrT78v=zQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202401181146.fuoeskfzriq7%40alvherre.pgsql>
Precedence: bulk

I've been eyeballing the coverage report generated after applying all
patches (but I only checked the code added by the 0008 patch).  AFAICS
the coverage is pretty good.  Some uncovered paths:

commands/explain.c (Hmm, I think this is a preexisting bug actually)

    3893          18 :         case T_TableFuncScan:
    3894          18 :             Assert(rte->rtekind == RTE_TABLEFUNC);
    3895          18 :             if (rte->tablefunc)
    3896           0 :                 if (rte->tablefunc->functype == TFT_XMLTABLE)
    3897           0 :                     objectname = "xmltable";
    3898             :                 else            /* Must be TFT_JSON_TABLE */
    3899           0 :                     objectname = "json_table";
    3900             :             else
    3901          18 :                 objectname = NULL;
    3902          18 :             objecttag = "Table Function Name";
    3903          18 :             break;

parser/gram.y:

   16940             : json_table_plan_cross:
   16941             :             json_table_plan_primary CROSS json_table_plan_primary
   16942          39 :                 { $$ = makeJsonTableJoinedPlan(JSTPJ_CROSS, $1, $3, @1); }
   16943             :             | json_table_plan_cross CROSS json_table_plan_primary
   16944           0 :                 { $$ = makeJsonTableJoinedPlan(JSTPJ_CROSS, $1, $3, @1); }
Not really sure how critical this one is TBH.


utils/adt/jsonpath_exec.c:

    3492             : /* Recursively reset scan and its child nodes */
    3493             : static void
    3494         120 : JsonTableRescanRecursive(JsonTablePlanState * state)
    3495             : {
    3496         120 :     if (state->type == JSON_TABLE_JOIN_STATE)
    3497             :     {
    3498           0 :         JsonTableJoinState *join = (JsonTableJoinState *) state;
    3499             : 
    3500           0 :         JsonTableRescanRecursive(join->left);
    3501           0 :         JsonTableRescanRecursive(join->right);
    3502           0 :         join->advanceRight = false;
    3503             :     }

I think this one had better be covered.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"The saddest aspect of life right now is that science gathers knowledge faster
 than society gathers wisdom."  (Isaac Asimov)