Horton

Thread

Commits

  1. Update query_id computation

  1. Some oversights in query_id calculation

    Julien Rouhaud <rjuju123@gmail.com> — 2021-04-25T08:11:19Z

    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

  2. Re: Some oversights in query_id calculation

    Aleksander Alekseev <aleksander@timescale.com> — 2021-04-28T10:19:36Z

    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?

[1]: https://github.com/afiskon/pgscripts

-- 
Best regards,
Aleksander Alekseev

  3. Re: Some oversights in query_id calculation

    Julien Rouhaud <rjuju123@gmail.com> — 2021-04-28T10:27:41Z

    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

  4. Re: Some oversights in query_id calculation

    Aleksander Alekseev <aleksander@timescale.com> — 2021-04-28T12:22:39Z

    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 = 1
>
> You should see failures doing a check-world or simply a make -C
> contrib/pg_stat_statements check
>


-- 
Best regards,
Aleksander Alekseev

  5. Re: Some oversights in query_id calculation

    Julien Rouhaud <rjuju123@gmail.com> — 2021-05-02T04:27:37Z

    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 check
> 
> Sorry, 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!

  6. Re: Some oversights in query_id calculation

    Bruce Momjian <bruce@momjian.us> — 2021-05-03T18:59:42Z

    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 check
> > 
> > Sorry, 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.

  7. Re: Some oversights in query_id calculation

    Julien Rouhaud <rjuju123@gmail.com> — 2021-05-05T00:33:36Z

    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 check
> > > 
> > > Sorry, 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!