čt 22. 12. 2022 v 17:15 odesílatel Dmitry Dolgov <9erthali...@gmail.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 >