> > > I agree that ComputeConstantLengths should be in core. This one is > > > not a gray area IMO. The query jumble already records constant locations, > > > but leaves the lengths unset. ComputeConstantLengths is just the > > > completion of that work. There could be no other interpretation, > > > unlike generate_normalized_query, of what the lengths should be. > > > > This is an interesting remark. One problem with any patches presented yet > > is that we don’t attach the lengths as part of the in-core query jumbling > > procedure: we plug the lengths later using the same structure as the query > > jumbling. It seems to me that this is half-baked overall. I think that we > > don’t want to pay the extra length computation in the core query jumbling > > at the end, then why does it make sense to include the lengths in the > > JumbleState structure at all? Shouldn’t we use a different structure filled > > inside PGSS for this purpose rather than reuse the same thing for PGSS and > > the in-core part (don’t have the code in front of me now). > > I see where you're coming from on that, but I don't think we can > remove anything here in practice:
Yes. Not unless we want to rely on the parser to track the lengths in Const, which could be invasive, but I have not looked into it. Paying the length computation at the end of every jumble is also going to impact performance, so that will not be a good idea. > I still think it'd be reasonable for us to include > ComputeConstantLengths in core to complete the picture of what we're > doing with _jumbleElements and the length field already anyway. Its > basically a way to fully hydrate the partially filled out JumbleState > from the initial jumble. I fully agree, ComputeConstantLengths is an optional post-jumble-query step for a consumer that wishes to calculate the lengths. The length calculation is not unique to a plug-in, so in my mind the work it's doing is core jumbling functionality. -- Sami Imseih Amazon Web Services (AWS)
