Hi, On 2022-06-29 11:40:45 +1200, David Rowley wrote: > On Sat, 18 Jun 2022 at 08:06, Andres Freund <and...@anarazel.de> wrote: > > I also attached my heavily-WIP patches for the ExprEvalStep issues, I > > accidentally had only included a small part of the contents of the json fix. > > I've now looked at the 0003 patch. I like the idea you have about > moving some of the additional fields into ScalarArrayOpExprHashTable. > I think the patch can even go a little further and move the hash_finfo > into there too. This means we don't need to dereference the "op" in > saop_element_hash().
Makes sense. > To make this work, I did need to tag the ScalarArrayOpExpr into the > ExprEvalStep. That's required now since some of the initialization of > the hash function fields is delayed until > ExecEvalHashedScalarArrayOp(). We need to know the > ScalarArrayOpExpr's hashfuncid and inputcollid. Makes sense. > Another small thing which I considered doing was to put the > hash_fcinfo_data field as the final field in > ScalarArrayOpExprHashTable so that we could allocate the memory for > the hash_fcinfo_data in the same allocation as the > ScalarArrayOpExprHashTable. This would reduce the pointer > dereferencing done in saop_element_hash() a bit further. I just > didn't notice anywhere else where we do that for FunctionCallInfo, so > I resisted doing this. I think that'd make sense - it does add a bit of size calculation magic, but it shouldn't be a problem. I'm fairly sure we do this in other parts of the code. > (There was also a small bug in your patch where you mistakenly cast to > an OpExpr instead of ScalarArrayOpExpr when you were fetching the > inputcollid) Ooops. Are you good pushing this? I'm fine with you doing so wether you adapt it further or not. Greetings, Andres Freund
