Dmitry Dolgov <9erthali...@gmail.com> writes: > And now for something completely different, here is a new patch version. > It contains a small fix for one problem we've found during testing (one > path code was incorrectly assuming find_const_walker results).
I've been saying from day one that pushing the query-hashing code into the core was a bad idea, and I think this patch perfectly illustrates why. We can debate whether the rules proposed here are good for pg_stat_statements or not, but it seems inevitable that they will be a disaster for some other consumers of the query hash. In particular, dropping external parameters from the hash seems certain to break something for somebody --- do you really think that a query with two int parameters is equivalent to one with five float parameters for all query-identifying purposes? I can see the merits of allowing different numbers of IN elements to be considered equivalent for pg_stat_statements, but this patch seems to go far beyond that basic idea, and I fear the side-effects will be very bad. Also, calling eval_const_expressions in the query jumbler is flat out unacceptable. There is way too much code that could be reached that way (more or less the entire executor, to start with). I don't have a lot of faith that it'd never modify the input tree, either. regards, tom lane