Some oversights in query_id calculation
Hi,
While doing some sanity checks on the regression tests, I found some queries
that are semantically different but end up with identical query_id.
Two are an old issues:
- the "ONLY" in FROM [ONLY] isn't hashed
- the agglevelsup field in GROUPING isn't hashed
Another one was introduced in pg13 with the WITH TIES not being hashed.
The last one new in pg14: the "DISTINCT" in "GROUP BY [DISTINCT]" isn't hash.
I'm attaching a patch that fixes those, with regression tests to reproduce each
problem.
There are also 2 additional debatable cases on whether this is a semantic
difference or not:
- aliases aren't hashed. That's usually not a problem, except when you use
row_to_json(), since you'll get different keys
- the NAME in XmlExpr (eg: xmlpi(NAME foo,...)) isn't hashed, so you generate
different elements
Attachments:
v1-0001-Fix-some-oversights-in-query_id-calculation.patchtext/x-diff; charset=us-asciiDownload+207-1
Hi Julien,
I'm attaching a patch that fixes those, with regression tests to reproduce each
problem.
I believe something could be not quite right with the patch. Here is what I did:
$ git apply ...
# revert the changes in the code but keep the new tests
$ git checkout src/backend/utils/misc/queryjumble.c
$ ./full-build.sh && single-install.sh && make installcheck-world
... where named .sh scripts are something I use to quickly check a patch [1]https://github.com/afiskon/pgscripts.
I was expecting that several tests will fail but they didn't. Maybe I
missed something?
[1]: https://github.com/afiskon/pgscripts
--
Best regards,
Aleksander Alekseev
Hi Aleksander,
On Wed, Apr 28, 2021 at 01:19:36PM +0300, Aleksander Alekseev wrote:
Hi Julien,
I'm attaching a patch that fixes those, with regression tests to reproduce each
problem.I believe something could be not quite right with the patch. Here is what I did:
$ git apply ...
# revert the changes in the code but keep the new tests
$ git checkout src/backend/utils/misc/queryjumble.c
$ ./full-build.sh && single-install.sh && make installcheck-world... where named .sh scripts are something I use to quickly check a patch [1].
I was expecting that several tests will fail but they didn't. Maybe I
missed something?
I think it's because installcheck-* don't run pg_stat_statements' tests, see
its Makefile:
# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
# which typical installcheck users do not have (e.g. buildfarm clients).
NO_INSTALLCHECK = 1
You should see failures doing a check-world or simply a make -C
contrib/pg_stat_statements check
Hi Julien,
You should see failures doing a check-world or simply a make -C
contrib/pg_stat_statements check
Sorry, my bad. I was running make check-world, but did it with -j4 flag
which was a mistake.
The patch is OK.
On Wed, Apr 28, 2021 at 1:27 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
Hi Aleksander,
On Wed, Apr 28, 2021 at 01:19:36PM +0300, Aleksander Alekseev wrote:
Hi Julien,
I'm attaching a patch that fixes those, with regression tests to
reproduce each
problem.
I believe something could be not quite right with the patch. Here is
what I did:
$ git apply ...
# revert the changes in the code but keep the new tests
$ git checkout src/backend/utils/misc/queryjumble.c
$ ./full-build.sh && single-install.sh && make installcheck-world... where named .sh scripts are something I use to quickly check a patch
[1].
I was expecting that several tests will fail but they didn't. Maybe I
missed something?I think it's because installcheck-* don't run pg_stat_statements' tests,
see
its Makefile:# Disabled because these tests require
"shared_preload_libraries=pg_stat_statements",
# which typical installcheck users do not have (e.g. buildfarm clients).
NO_INSTALLCHECK = 1You should see failures doing a check-world or simply a make -C
contrib/pg_stat_statements check
--
Best regards,
Aleksander Alekseev
Hi Aleksander,
On Wed, Apr 28, 2021 at 03:22:39PM +0300, Aleksander Alekseev wrote:
Hi Julien,
You should see failures doing a check-world or simply a make -C
contrib/pg_stat_statements checkSorry, my bad. I was running make check-world, but did it with -j4 flag
which was a mistake.The patch is OK.
Thanks for reviewing!
On Sun, May 2, 2021 at 12:27:37PM +0800, Julien Rouhaud wrote:
Hi Aleksander,
On Wed, Apr 28, 2021 at 03:22:39PM +0300, Aleksander Alekseev wrote:
Hi Julien,
You should see failures doing a check-world or simply a make -C
contrib/pg_stat_statements checkSorry, my bad. I was running make check-world, but did it with -j4 flag
which was a mistake.The patch is OK.
Thanks for reviewing!
Patch applied, thanks.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
On Mon, May 03, 2021 at 02:59:42PM -0400, Bruce Momjian wrote:
On Sun, May 2, 2021 at 12:27:37PM +0800, Julien Rouhaud wrote:
Hi Aleksander,
On Wed, Apr 28, 2021 at 03:22:39PM +0300, Aleksander Alekseev wrote:
Hi Julien,
You should see failures doing a check-world or simply a make -C
contrib/pg_stat_statements checkSorry, my bad. I was running make check-world, but did it with -j4 flag
which was a mistake.The patch is OK.
Thanks for reviewing!
Patch applied, thanks.
Thanks a lot Bruce!