Re: JIT performance bug/regression & JIT EXPLAIN

Hi,

On 2019-10-28 11:27:02 -0700, Maciek Sakrejda wrote:
> >But that's pretty crappy, because it means that the 'shape' of the output depends on the jit_details option.
> 
> Yeah, that would be hard to work with. What about adding it as a sibling group?
> 
> "Filter": "(lineitem.l_shipdate <= '1998-09-18 00:00:00'::timestamp
> without time zone)",
> "Filter JIT": {
>   "Expr": "evalexpr_0_2",
>   "Deform Scan": "deform_0_3",
>   "Deform Outer": null,
>   "Deform Inner": null
> }
> 
> Also not that pretty, but at least it's easier to work with

What I dislike about that is that it basically again is introducing
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.


> (I also
> changed the dashes to spaces since that's what the rest of EXPLAIN is
> doing as a matter of style).

That makes sense.


> >But the compat break due to that change is not small- perhaps we could instead mark that in another way?
> 
> We could add a "Projects" boolean key instead? Of course that's more
> awkward in text mode. Maybe compat break is less of an issue in text
> mode and we can treat this differently?

Yea, I think projects as a key for each node makes sense. For text mode
I guess we could just display the key on the same line when es->verbose
is set?  Still not sure if not just changing the output is the better
approach.

Another alternative would be to just remove the 'Output' line when a
node doesn't project - it can't really carry meaning in those cases
anyway?


> >For HashJoin/Hash I've added 'Outer Hash Key' and 'Hash Key' for each key, but only in verbose mode.
> 
> That reads pretty well to me. What does the structured output look
> like?

Just a new "Outer Hash Key" for the HashJoin node, and "Hash Key" for
the Hash node. Perhaps the latter should be 'Inner Hash Key' - while
that's currently a bit confusing because of Hash's subtree being the
outer tree, it'd reduce changes when merging Hash into HashJoin [1], and
it's clearer when looking at the HashJoin node itself.

Here's an example query:

EXPLAIN (VERBOSE, FORMAT JSON, COSTS OFF) SELECT pc.oid::regclass, pc.relkind, pc.relfilenode, pc_t.oid::regclass as
toast_rel,pc_t.relfilenode as toast_relfilenode FROM pg_class pc LEFT OUTER JOIN pg_class pc_t ON (pc.reltoastrelid =
pc_t.oid);
[
  {
    "Plan": {
      "Node Type": "Hash Join",
      "Parallel Aware": false,
      "Join Type": "Left",
      "Project": ["(pc.oid)::regclass", "pc.relkind", "pc.relfilenode", "(pc_t.oid)::regclass", "pc_t.relfilenode"],
      "Inner Unique": true,
      "Hash Cond": "(pc.reltoastrelid = pc_t.oid)",
      "Outer Hash Key": "pc.reltoastrelid",
      "Plans": [
        {
          "Node Type": "Seq Scan",
          "Parent Relationship": "Outer",
          "Parallel Aware": false,
          "Relation Name": "pg_class",
          "Schema": "pg_catalog",
          "Alias": "pc",
          "Output": ["pc.oid", "pc.relname", "pc.relnamespace", "pc.reltype", "pc.reloftype", "pc.relowner",
"pc.relam","pc.relfilenode", "pc.reltablespace", "pc.relpages", "pc.reltuples", "pc.relallvisible", "pc.reltoastrelid",
"pc.relhasindex","pc.relisshared", "pc.relpersistence", "pc.relkind", "pc.relnatts", "pc.relchecks", "pc.relhasrules",
"pc.relhastriggers","pc.relhassubclass", "pc.relrowsecurity", "pc.relforcerowsecurity", "pc.relispopulated",
"pc.relreplident","pc.relispartition", "pc.relrewrite", "pc.relfrozenxid", "pc.relminmxid", "pc.relacl",
"pc.reloptions","pc.relpartbound"]
 
        },
        {
          "Node Type": "Hash",
          "Parent Relationship": "Inner",
          "Parallel Aware": false,
          "Output": ["pc_t.oid", "pc_t.relfilenode"],
          "Hash Key": "pc_t.oid",
          "Plans": [
            {
              "Node Type": "Seq Scan",
              "Parent Relationship": "Outer",
              "Parallel Aware": false,
              "Relation Name": "pg_class",
              "Schema": "pg_catalog",
              "Alias": "pc_t",
              "Project": ["pc_t.oid", "pc_t.relfilenode"]
            }
          ]
        }
      ]
    }
  }
]

and in plain text:

Hash Left Join
  Project: (pc.oid)::regclass, pc.relkind, pc.relfilenode, (pc_t.oid)::regclass, pc_t.relfilenode
  Inner Unique: true
  Hash Cond: (pc.reltoastrelid = pc_t.oid)
  Outer Hash Key: pc.reltoastrelid
  ->  Seq Scan on pg_catalog.pg_class pc
        Output: pc.oid, pc.relname, pc.relnamespace, pc.reltype, pc.reloftype, pc.relowner, pc.relam, pc.relfilenode,
pc.reltablespace,pc.relpages, pc.reltuples, pc.relallvisible, pc.reltoastrelid, pc.relhasindex, pc.relisshared,
pc.relpersistence,pc.relkind, pc.relnatts, pc.relchecks, pc.relhasrules, pc.relhastriggers, pc.relhassubclass,
pc.relrowsecurity,pc.relforcerowsecurity, pc.relispopulated, pc.relreplident, pc.relispartition, pc.relrewrite,
pc.relfrozenxid,pc.relminmxid, pc.relacl, pc.reloptions, pc.relpartbound
 
  ->  Hash
        Output: pc_t.oid, pc_t.relfilenode
        Hash Key: pc_t.oid
        ->  Seq Scan on pg_catalog.pg_class pc_t
              Project: pc_t.oid, pc_t.relfilenode

which also serves as an example about my previous point about
potentially just hiding the 'Output: ' bit when no projection is done:
It's very verbose, without adding much, while hiding that there's
actually nothing being done at the SeqScan level.

I've attached a rebased version of the patcheset. No changes except for
a minor conflict, and removing some already applied bugfixes.

Greetings,

Andres Freund

[1] https://www.postgresql.org/message-id/20191028231526.wcnwag7lllkra4qt%40alap3.anarazel.de