> On Tue, May 20, 2025 at 06:30:25AM GMT, Michael Paquier wrote: > On Mon, May 12, 2025 at 06:40:43PM -0400, Sami Imseih wrote: > > Also, LocationExpr is not really an expression node, but a wrapper to > > an expression node, so I think it's wrong to define it as a Node and be > > required to add the necessary handling for it in nodeFuncs.c. I think we > > can just define it as a struct in gram.y so it can carry the locations of > > the > > expression and then set the List of the location boundaries in > > A_Expr and A_ArrayExpr. right? > > Right. LocationExpr is not a full Node, so if we can do these > improvements without it we have less maintenance to worry about across > the board with less code paths. At the end, I think that we should > try to keep the amount of work done by PGSS as minimal as possible. > > 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 > that we sometimes do not want to push down to the code calling the > parser. If we can do the same, the result could be nicer.
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? At the same time AFAICT there isn't much more code paths to worry about in case of a LocationExpr as a node -- in the end all options would have to embed the location information into ArrayExpr during transformation, independently from how this information was conveyed. Aside that the only extra code we've got is node functions (exprType, etc). Is there anything I'm missing here? > By the way, the new test cases for ARRAY lists are sent in the last > patch of the series posted on this thread: > https://www.postgresql.org/message-id/7zbzwk4btnxoo4o3xbtzefoqvht54cszjj4bol22fmej5nmgkf@dbcn4wtakw4y > > These should be first in the list, IMO, so as it is possible to track > what the behavior was before the new logic as of HEAD, and what the > behavior would become after the new logic. Sure, I can reshuffle that. BTW, I'm going to be away for a couple of weeks soon. So if you want to decide one way or another soonish, let's do it now.
