From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Duplicate node tag assignments |
Date: | 2016-12-29 03:57:10 |
Message-ID: | CAA4eK1J6Ed3G-bQXQ-4DSScLJmByY1JLOW8dLb2YpcMkfcYVPw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Dec 28, 2016 at 10:03 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> By chance I happened to notice that the recent partition patch pushed
> us over the number of available node tags between
>
> T_A_Expr = 900,
>
> and
>
> T_TriggerData = 950, /* in commands/trigger.h */
>
> Specifically we now have some of the replication grammar node type
> codes conflicting with some of the "random other stuff" tags.
> It's not terribly surprising that that hasn't caused visible problems,
> because of the relatively localized use of replication grammar nodes,
> but it's still a Bad Thing. And it's especially bad that apparently
> no one's compiler has complained about it.
>
> So we need to renumber enum NodeTag a bit, which is no big deal,
> but I think we had better take thought to prevent a recurrence
> (which might have worse consequences next time).
>
> One idea is to add StaticAsserts someplace asserting that there's
> still daylight at each manually-assigned break in the NodeTag list.
> But I'm not quite sure how to make that work. For example, asserting
> that T_PlanInvalItem < T_PlanState won't help if people add new nodes
> after PlanInvalItem and don't think to update the Assert.
>
> Or we could just abandon the manually-assigned breaks in the list
> altogether, and let NodeTags run from 1 to whatever. This would
> slightly complicate debugging, in that the numeric values of node
> tags would change more than they used to, but on the whole that does
> not sound like a large problem.
>
Yeah. In most cases, during debugging I use the tag for typecasting
the node to see the values of the particular node type.
> When you're working in gdb, say,
> it's easy enough to convert:
>
> (gdb) p (int) T_CreateReplicationSlotCmd
> $8 = 950
> (gdb) p (enum NodeTag) 949
> $9 = T_BaseBackupCmd
>
> So I'm leaning to the second, more drastic, solution.
>
Sounds sensible.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2016-12-29 04:08:45 | Re: Duplicate node tag assignments |
Previous Message | Dilip Kumar | 2016-12-29 03:42:55 | Re: Proposal : Parallel Merge Join |