Re: Remove Value node struct

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, ilmari(at)ilmari(dot)org
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Remove Value node struct
Date: 2021-09-07 09:22:24
Message-ID: 720c943c-71c8-8704-dddf-3378387db87d@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 30.08.21 04:13, Kyotaro Horiguchi wrote:
>> However, the patch adds:
>>
>>> +typedef struct Null
>>> +{
>>> + NodeTag type;
>>> + char *val;
>>> +} Null;
>>
>> which doesn't seem to be used anywhere. Is that a leftoverf from an
>> intermediate development stage?
>
> +1 Looks like so, it can be simply removed.

fixed

> 0002:
> there's an "integer Value node" in gram.y: 7776.

fixed

> - n = makeFloatConst(v->val.str, location);
> + n = (Node *) makeFloatConst(castNode(Float, v)->val, location);
>
> makeFloatConst is Node* so the cast doesn't seem needed. The same can
> be said for Int and String Consts. This looks like a confustion with
> makeInteger and friends.

fixed

> + else if (IsA(obj, Integer))
> + _outInteger(str, (Integer *) obj);
> + else if (IsA(obj, Float))
> + _outFloat(str, (Float *) obj);
>
> I felt that the type enames are a bit confusing as they might be too
> generic, or too close with the corresponding binary types.
>
>
> - Node *arg; /* a (Value *) or a (TypeName *) */
> + Node *arg;
>
> Mmm. It's a bit pity that we lose the generic name for the value nodes.

Not sure what you mean here.

Attachment Content-Type Size
v2-0001-Remove-useless-casts.patch text/plain 9.2 KB
v2-0002-Remove-Value-node-struct.patch text/plain 56.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2021-09-07 09:24:39 Re: Avoid stuck of pbgench due to skipped transactions
Previous Message houzj.fnst@fujitsu.com 2021-09-07 09:14:31 RE: Added missing invalidations for all tables publication