Re: Remove Value node struct

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: ilmari(at)ilmari(dot)org
Cc: peter(dot)eisentraut(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Remove Value node struct
Date: 2021-08-30 02:13:58
Message-ID: 20210830.111358.1755070997683103894.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Agree to the motive and +1 for the concept.

At Wed, 25 Aug 2021 15:00:13 +0100, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org> wrote in
> 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.

0001 looks fine to me.

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

- 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.

+ 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.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2021-08-30 02:14:17 Re: row filtering for logical replication
Previous Message Masahiko Sawada 2021-08-30 01:12:07 Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)