Re: wrong query result with jit_above_cost= 0

From: Andres Freund <andres(at)anarazel(dot)de>
To: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
Cc: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: wrong query result with jit_above_cost= 0
Date: 2018-07-23 00:09:37
Message-ID: 20180723000937.7xstiiq7ugfs6iqv@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2018-07-07 21:41:05 +0200, Dmitry Dolgov wrote:
> Ok, looks like I found the issue. v_aggcontext & v_curaggcontext have nothing
> to do here (this change was just masking a problem by changing a memory context
> so that the wrong one will never be used). The problem was that in the
> llvmjit_expr in AGG_INIT_TRANS section we need to assign a current memory
> context from op->d.agg_init_trans.aggcontext (see the attached patch),
> otherwise we'll get in the situation when current memory context is hashcontext
> instead of aggcontext.

Nice catch! I pushed your fix, but I also made it set current_set.

> Also, I found one suspicious thing, in AGG_PLAIN_TRANS section we don't
> switch the memory context back in the branch with ExecAggTransReparent. I
> never found any consequences of that, but just in case I believe it makes sense
> to do so.

I'll look at that next.

> And the last thing - where can I find a documentation about how to properly
> apply patches for GDB & perf support to llvm? I remember they were posted here,
> and found some of them here [1] from Andres, but apparently part of them was
> already applied on top of llvm. Looks like for the gdb support I need to apply
> 0006-ORC-JIT-event-listener-support (since there is a gdb listener mentioned
> there), but with this patch I have an error:
>
> error: ‘ObjHandleT’ was not declared in this scope
>
> So I'm confused how should it be?

I've merged the GDB part into LLVM, and am about to merge the perf part
too. I plan to push a fix to PG adapting it to use the agreed upon /
merged LLVM APIs. Then you'll just need a recent LLVM
checkout. Unfortunately the relevant LLVM internal APIs have changed
quite rapidly over the last few releases (and a lot within individual
releases), so it's not easy to provide a patch for the individual versions.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-07-23 00:41:06 Re: More consistency for some file-related error message
Previous Message Fabien COELHO 2018-07-22 23:14:53 pgbench - remove double declaration of hash functions