| 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: | Whole Thread | Raw Message | 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 |
| 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 |