Re: JIT performance bug/regression & JIT EXPLAIN - Mailing list pgsql-hackers

From Andres Freund
Subject Re: JIT performance bug/regression & JIT EXPLAIN
Date
Msg-id 20191112222131.4ueqjn7kktxxlkfo@alap3.anarazel.de
Whole thread Raw
In response to Re: JIT performance bug/regression & JIT EXPLAIN  (Maciek Sakrejda <m.sakrejda@gmail.com>)
List pgsql-hackers
Hi,

On 2019-11-12 13:42:10 -0800, Maciek Sakrejda wrote:
> On Mon, Oct 28, 2019 at 5:02 PM Andres Freund <andres@anarazel.de> wrote:
> > What I dislike about that is that it basically again is introducing
> 
> "again"? Am I missing some history here? I'd love to read up on this
> if there are mistakes to learn from.

I think I was mostly referring to mistakes we've made for the json etc
key names. By e.g. having expressions as "Function Call", "Table
Function Call", "Filter", "TID Cond", ... a tool that wants to interpret
the output needs awareness of all of these different names, rather than
knowing that everything with a sub-group "Expression" has to be an
expression.

I.e. instead of

    "Plan": {
      "Node Type": "Seq Scan",
      "Parallel Aware": false,
      "Relation Name": "pg_class",
      "Schema": "pg_catalog",
      "Alias": "pg_class",
      "Startup Cost": 0.00,
      "Total Cost": 17.82,
      "Plan Rows": 385,
      "Plan Width": 68,
      "Output": ["relname", "tableoid"],
      "Filter": "(pg_class.relname <> 'foo'::name)"
    }

we ought to have gone for

    "Plan": {
      "Node Type": "Seq Scan",
      "Parallel Aware": false,
      "Relation Name": "pg_class",
      "Schema": "pg_catalog",
      "Alias": "pg_class",
      "Startup Cost": 0.00,
      "Total Cost": 17.82,
      "Plan Rows": 385,
      "Plan Width": 68,
      "Output": ["relname", "tableoid"],
      "Filter": {"Expression" : { "text": (pg_class.relname <> 'foo'::name)"}}
    }

or something like that. Which'd then make it obvious how to add
information about JIT to each expression.


Whereas the proposal of the separate key name perpetuates the
messiness...


> > something that requires either pattern matching on key names (i.e. a key
> > of '(.*) JIT' is one that has information about JIT, and the associated
> > expresssion is in key $1), or knowing all the potential keys an
> > expression could be in.
> 
> That still seems less awkward than having to handle a Filter field
> that's either scalar or a group.

Yea, it's a sucky option :(


> Most current EXPLAIN options just add
> additional fields to the structured plan instead of modifying it, no?
> If that output is better enough, though, maybe we should just always
> make Filter a group and go with the breaking change? If tooling
> authors need to treat this case specially anyway, might as well evolve
> the format.

Yea, maybe that's the right thing to do. Would be nice to have some more
input...

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: "Jonathan S. Katz"
Date:
Subject: 2019-11-14 Press Release Draft
Next
From: Tom Lane
Date:
Subject: Re: make pg_attribute_noreturn() work for msvc?