Re: Schema variables - new implementation for Postgres 15 (typo)

čt 22. 12. 2022 v 17:15 odesílatel Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
napsal:

> Hi,
>
> I'm continuing review the patch slowly, and have one more issue plus one
> philosophical question.
>
> The issue have something to do with variables invalidation. Enabling
> debug_discard_caches = 1 (about which I've learned from this thread) and
> running this subset of the test suite:
>
> CREATE SCHEMA svartest;
> SET search_path = svartest;
> CREATE VARIABLE var3 AS int;
>
> CREATE OR REPLACE FUNCTION inc(int)
> RETURNS int AS $$
> BEGIN
> LET svartest.var3 = COALESCE(svartest.var3 + $1, $1);
> RETURN var3;
> END;
> $$ LANGUAGE plpgsql;
>
> SELECT inc(1);
> SELECT inc(1);
> SELECT inc(1);
>
> crashes in my setup like this:
>
> #2 0x0000000000b432d4 in ExceptionalCondition
> (conditionName=0xce9b99 "n >= 0 && n < list->length", fileName=0xce9c18
> "list.c", lineNumber=770) at assert.c:66
> #3 0x00000000007d3acd in list_delete_nth_cell (list=0x18ab248,
> n=-3388) at list.c:770
> #4 0x00000000007d3b88 in list_delete_cell (list=0x18ab248,
> cell=0x18dc028) at list.c:842
> #5 0x00000000006bcb52 in sync_sessionvars_all (filter_lxid=true)
> at session_variable.c:524
> #6 0x00000000006bd4cb in SetSessionVariable (varid=16386,
> value=2, isNull=false) at session_variable.c:844
> #7 0x00000000006bd617 in SetSessionVariableWithSecurityCheck
> (varid=16386, value=2, isNull=false) at session_variable.c:885
> #8 0x00007f763b890698 in exec_stmt_let (estate=0x7ffcc6fd5190,
> stmt=0x18aa920) at pl_exec.c:5030
> #9 0x00007f763b88a746 in exec_stmts (estate=0x7ffcc6fd5190,
> stmts=0x18aaaa0) at pl_exec.c:2116
> #10 0x00007f763b88a247 in exec_stmt_block (estate=0x7ffcc6fd5190,
> block=0x18aabf8) at pl_exec.c:1935
> #11 0x00007f763b889a49 in exec_toplevel_block
> (estate=0x7ffcc6fd5190, block=0x18aabf8) at pl_exec.c:1626
> #12 0x00007f763b8879df in plpgsql_exec_function (func=0x18781b0,
> fcinfo=0x18be110, simple_eval_estate=0x0, simple_eval_resowner=0x0,
> procedure_resowner=0x0, atomic=true) at pl_exec.c:615
> #13 0x00007f763b8a2320 in plpgsql_call_handler (fcinfo=0x18be110)
> at pl_handler.c:277
> #14 0x0000000000721716 in ExecInterpExpr (state=0x18bde28,
> econtext=0x18bd3d0, isnull=0x7ffcc6fd56d7) at execExprInterp.c:730
> #15 0x0000000000723642 in ExecInterpExprStillValid
> (state=0x18bde28, econtext=0x18bd3d0, isNull=0x7ffcc6fd56d7) at
> execExprInterp.c:1855
> #16 0x000000000077a78b in ExecEvalExprSwitchContext
> (state=0x18bde28, econtext=0x18bd3d0, isNull=0x7ffcc6fd56d7) at
> ../../../src/include/executor/executor.h:344
> #17 0x000000000077a7f4 in ExecProject (projInfo=0x18bde20) at
> ../../../src/include/executor/executor.h:378
> #18 0x000000000077a9dc in ExecResult (pstate=0x18bd2c0) at
> nodeResult.c:136
> #19 0x0000000000738ea0 in ExecProcNodeFirst (node=0x18bd2c0) at
> execProcnode.c:464
> #20 0x000000000072c6e3 in ExecProcNode (node=0x18bd2c0) at
> ../../../src/include/executor/executor.h:262
> #21 0x000000000072f426 in ExecutePlan (estate=0x18bd098,
> planstate=0x18bd2c0, use_parallel_mode=false, operation=CMD_SELECT,
> sendTuples=true, numberTuples=0, direction=ForwardScanDirection,
> dest=0x18b3eb8, execute_once=true) at execMain.c:1691
> #22 0x000000000072cf76 in standard_ExecutorRun
> (queryDesc=0x189c688, direction=ForwardScanDirection, count=0,
> execute_once=true) at execMain.c:423
> #23 0x000000000072cdb3 in ExecutorRun (queryDesc=0x189c688,
> direction=ForwardScanDirection, count=0, execute_once=true) at
> execMain.c:367
> #24 0x000000000099afdc in PortalRunSelect (portal=0x1866648,
> forward=true, count=0, dest=0x18b3eb8) at pquery.c:927
> #25 0x000000000099ac99 in PortalRun (portal=0x1866648,
> count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x18b3eb8,
> altdest=0x18b3eb8, qc=0x7ffcc6fd5a70) at pquery.c:771
> #26 0x000000000099487d in exec_simple_query
> (query_string=0x17edcc8 "SELECT inc(1);") at postgres.c:1238
>
> It seems that sync_sessionvars_all tries to remove a cell that either
> doesn't
> belong to the xact_recheck_varids or weird in some other way:
>
> +>>> p l - xact_recheck_varids->elements
> $81 = -3388
>

I am able to repeat this issue. I'll look at it.

>
> The second thing I wanted to ask about is a more strategical question. Does
> anyone have clear understanding where this patch is going? The thread is
> quite
> large, and it's hard to catch up with all the details -- it would be great
> if
> someone could summarize the current state of things, are there any
> outstanding
> design issues or not addressed concerns?
>

I hope I fixed the issues founded by Julian and Tomas. Now there is not
implemented transaction support related to values, and I plan to implement
this feature in the next stage.
It is waiting for review.

>
> From the first look it seems some major topics the discussion is evolving
> are about:
>
> * Validity of the use case. Seems to be quite convincingly addressed in
> [1] and
> [2].
>
> * Complicated logic around invalidation, concurrent create/drop etc. (I
> guess
> the issue above is falling into the same category).
>
> * Concerns that session variables could repeat some problems of temporary
> tables.
>

Why do you think so? The variable has no mvcc support - it is just stored
value with local visibility without mvcc support. There can be little bit
similar issues like with global temporary tables.

>
> Is there anything else?
>
> [1]:
> https://www.postgresql.org/message-id/CAFj8pRBT-bRQJBqkzon7tHcoFZ1byng06peZfZa0G72z46YFxg%40mail.gmail.com
> [2]:
> https://www.postgresql.org/message-id/flat/CAFj8pRBHSAHdainq5tRhN2Nns62h9-SZi0pvNq9DTe0VM6M1%3Dg%40mail.gmail.com#4faccb978d60e9b0b5d920e1a8a05bbf
>