Re: [PATCH] Generic type subscription

From: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
Cc: Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Victor Wagner <vitus(at)wagner(dot)pp(dot)ru>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Subject: Re: [PATCH] Generic type subscription
Date: 2017-01-15 22:15:28
Message-ID: CAKNkYnzX2Cw9ui4hnvDKU8Hyfq9etbGstzsATbhhz9PEi64_1Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Yes, but it was related to the idea of having `ArrayRef` and `JsonbRef` nodes
> for specific types. Since now there is generic `SubscriptingRef` node, I think
> it should be ok.

Sorry I misunderstood it.

> Just to be clear - as far as I understood, these compilation problems were
> caused not because the extension knew something about ArrayRef node in
> particular, but because the extension tried to extract all nodes to generate
> code from them. It means any change will require "refetching", so I think it's
> natural for this extension.

Agree. It will be hard to maintain both nodes. And it is not so smart
to have two nodes ArrayRef (deprecated) and SubscriptingRef. It is not
hard to replace ArrayRef node with SubscriptingRef in other
extensions.

There is a little note:

> #include "utils/lsyscache.h"
> +#include "utils/syscache.h"
> #include "utils/memutils.h"

I think "utils/syscache.h" isn't necessary here. PostgreSQL could be
compiled without this include.

I suppose that this patch can be marked as "Ready for commiter". Any opinions?

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Petr Jelinek 2017-01-15 22:20:53 Re: Logical Replication WIP
Previous Message Jim Nasby 2017-01-15 21:01:47 FYI: git worktrees as replacement for "rsync the CVSROOT"