Re: remaining sql/json patches

On Sun, Apr 7, 2024 at 9:36 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
>
> 0002 needs an expanded commit message but I've run out of energy today.
>

some cosmetic issues in v51, 0002.

in struct JsonTablePathScan,
/* ERROR/EMPTY ON ERROR behavior */
bool errorOnError;

the comments seem not right.
I think "errorOnError" means
while evaluating the top level JSON path expression, whether "error on
error" is specified or not?

+ | NESTED <optional> PATH </optional> ]
<replaceable>json_path_specification</replaceable> <optional> AS
<replaceable>json_path_name</replaceable> </optional> COLUMNS (
<replaceable>json_table_column</replaceable> <optional>,
...</optional> )
</synopsis>

"NESTED <optional> PATH </optional> ] "
no need the closing bracket.

+ /* Update the nested plan(s)'s row(s) using this new row. */
+ if (planstate->nested)
+ {
+ JsonTableResetNestedPlan(planstate->nested);
+ if (JsonTablePlanNextRow(planstate->nested))
+ return true;
+ }
+
return true;
}

this part can be simplified as:
+ if (planstate->nested)
+{
+ JsonTableResetNestedPlan(planstate->nested);
+ JsonTablePlanNextRow(planstate->nested));
+}
since the last part, if it returns false, eventually it returns true.
also the comments seem slightly confusing?

v51 recursion function(JsonTablePlanNextRow, JsonTablePlanScanNextRow)
is far clearer than v50!
thanks. I think I get it.