Re: queryId constant squashing does not support prepared statements

> At the same time AFAICT there isn't much more code paths
> to worry about in case of a LocationExpr as a node

I can imagine there are others like value expressions,
row expressions, json array expressions, etc. that we may
want to also normalize.

> I believe it's worth to not only to keep amount of work to support
> LocationExpr as minimal as possible, but also impact on the existing
> code. What I see as a problem is keeping such specific information as
> the location boundaries in such a generic expression as A_Expr, where it
> will almost never be used. Do I get it right, you folks are ok with
> that?

There are other examples of fields that are minimally used in other structs.
Here is one I randomly spotted in SelectStmt such as SortClause, Limit options,
etc. I think the IN list is quite a common case, otherwise we would not
care as much as we do.

There are other examples of fields that are minimally used in other structs.
Here is one I randomly spotted in SelectStmt such as SortClause, Limit options,
etc. I think the IN list is quite a common case, otherwise we would not
care as much as we do. Adding another 8 bytes to the struts does not seem
like that big of a problem to me, especially the structs will remain below
64 bytes.


```
(gdb) ptype /o A_Expr
type = struct A_Expr {
/*      0      |       4 */    NodeTag type;
/*      4      |       4 */    A_Expr_Kind kind;
/*      8      |       8 */    List *name;
/*     16      |       8 */    Node *lexpr;
/*     24      |       8 */    Node *rexpr;
/*     32      |       4 */    ParseLoc location;
/* XXX  4-byte padding   */

                               /* total size (bytes):   40 */
                             }


(gdb) ptype \o A_ArrayExpr
Invalid character '\' in expression.
(gdb) ptype /o A_ArrayExpr
type = struct A_ArrayExpr {
/*      0      |       4 */    NodeTag type;
/* XXX  4-byte hole      */
/*      8      |       8 */    List *elements;
/*     16      |       4 */    ParseLoc location;
/* XXX  4-byte padding   */

                               /* total size (bytes):   24 */
                             }
```


In general, Making something like T_LocationExpr as a query node
seems totally wrong to me. It's not a node, but rather a temporary
wrapper of some location information and it does not seem it has
business being used by the time we get to thee expression
transformations. It seems very odd considering location information
are simple fields in the parse node itself.


>I was a bit worried about not using a Node but Sami has reminded me
> last week that we already have in gram.y the concept of using some
> private structures to track intermediate results done by the parsing

Attached is a sketch of what I mean. There is a private struct that tracks
the list boundaries and this can wrap in_expr or whatever else makes
sense in the future.

+typedef struct ListWithBoundary
+{
+       Node       *expr;
+       ParseLoc        start;
+       ParseLoc        end;
+} ListWithBoundary;
+
 /* ConstraintAttributeSpec yields an integer bitmask of these flags: */
 #define CAS_NOT_DEFERRABLE                     0x01
 #define CAS_DEFERRABLE                         0x02
@@ -269,6 +276,7 @@ static Node *makeRecursiveViewSelect(char
*relname, List *aliases, Node *query);
        struct KeyAction *keyaction;
        ReturningClause *retclause;
        ReturningOptionKind retoptionkind;
+       struct ListWithBoundary *listwithboundary;
 }


+%type <listwithboundary> in_expr

The values are then added to start_location/end_location ParseLoc in
A_ArrayExpr and A_Expr. Doing it this will keep changes to the parse_expr.c
code to a minimum, only the IN transformation will need to set the values
of the A_Expr into the final A_ArrayExpr.

--
Sami Imseih
Amazon Web Services (AWS)