Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate query IDs with the hint table #193

Closed
wants to merge 1 commit into from

Conversation

michaelpq
Copy link
Collaborator

The hint table is reworked so as the normalized query string is replaced by the query ID generated by PostgreSQL, based on the jumbling done by the backend. This has multiple advantages:

  • Less bloat in the hint table, as query strings are replaced by bigint to track the query ID associated with a hint.
  • Less work for the module. Query normalization consumes cycles to apply the constants to the string for the match in the hint table. This code was copy-pasted from pg_stat_statements, and there is no need for it anymore.
  • Less error-prone, as a type in the data inserted in the hint table could easily cause a mismatch fail when looking for an entry.

Regression tests are adjusted to cope with this patch, where a PL/PgSQL function is added to be able to retrieve the query ID from a query string, as a wrapper for EXPLAIN (VERBOSE, JSON FORMAT). The query IDs can vary across platforms, so these are hidden to keep the tests portable. The following tweaks are done to the regression tests:

  • pg_hint_plan.sql has been switched to use this wrapper, with no changes in the regression test coverage.
  • ut-A, similarly, switches to use the function to retrieve query IDs. A use-case was becoming useless, where we checked for two hints set for the same query ID computed.
  • ut-T included two tests with CTAS and DECLARE, which interact badly with the PL wrapper. These could be covered with pg_stat_statements, but this dependency is planned to be removed from this test suite, and they covered rather edge cases with limited values with the query ID integration.

The documentation is updated, and the code in charge of the query normalization that was inherited from pg_stat_statements is removed, shaving a good chunk of code. On version upgrade, the past table is removed, replaced by the new one. Using the hint table requires an upgrade up to 1.7.0.

The hint table is reworked so as the normalized query string is replaced
by the query ID generated by PostgreSQL, based on the jumbling done by
the backend.  This has multiple advantages:
- Less bloat in the hint table, as query strings are replaced by bigint
to track the query ID associated with a hint.
- Less work for the module.  Query normalization consumes cycles to
apply the constants to the string for the match in the hint table.  This
code was copy-pasted from pg_stat_statements, and there is no need for
it anymore.
- Less error-prone, as a type in the data inserted in the hint table
could easily cause a mismatch fail when looking for an entry.

Regression tests are adjusted to cope with this patch, where a PL/PgSQL
function is added to be able to retrieve the query ID from a query
string, as a wrapper for EXPLAIN (VERBOSE, JSON FORMAT).  The query IDs
can vary across platforms, so these are hidden to keep the tests
portable.  The following tweaks are done to the regression tests:
- pg_hint_plan.sql has been switched to use this wrapper, with no
changes in the regression test coverage.
- ut-A, similarly, switches to use the function to retrieve query IDs.
A use-case was becoming useless, where we checked for two hints set for
the same query ID computed.
- ut-T included two tests with CTAS and DECLARE, which interact badly
with the PL wrapper.  These could be covered with pg_stat_statements,
but this dependency is planned to be removed from this test suite, and
they covered rather edge cases with limited values with the query ID
integration.

The documentation is updated, and the code in charge of the query
normalization that was inherited from pg_stat_statements is removed,
shaving a good chunk of code.  On version upgrade, the past table is
removed, replaced by the new one.  Using the hint table requires an
upgrade up to 1.7.0.
michaelpq added a commit that referenced this pull request May 20, 2024
The hint table is reworked so as the normalized query string is replaced
by the query ID generated by PostgreSQL, based on the jumbling done by
the backend.  This has multiple advantages:
- Less bloat in the hint table, as query strings are replaced by bigint
to track the query ID associated with a hint.
- Less work for the module itself.  Query normalization consumes cycles
to apply the constants to the string for the match in the hint table.
The longer the string, the longer it takes to apply the normalization.
This code was copy-pasted from pg_stat_statements, and there is no need
for it anymore.  This also means less long-term technical debt to keep
this code in line with upstream.
- Less error-prone, as a type in the data inserted in the hint table
could easily cause a mismatch fail when looking for an entry.

Regression tests are adjusted to cope with this commit, where a PL/PgSQL
function is added, to be able to retrieve the query ID from a query
string, wrapped around EXPLAIN (VERBOSE, JSON FORMAT).  The query IDs
can vary across platforms, so these are hidden to keep the tests
portable.  The following tweaks are done to the regression tests:
- pg_hint_plan.sql has been switched to use this wrapper, with no
changes in the regression test coverage.
- ut-A, similarly, switches to use the function to retrieve query IDs.
A use-case was becoming useless, where we checked for two hints set for
the same query ID computed.
- ut-T included two tests with CTAS and DECLARE, which interact badly
with the PL wrapper.  These could be covered with pg_stat_statements,
but this dependency is planned to be removed entirely, and they covered
edge cases with limited value at the end.

The documentation is updated, and the code in charge of the query
normalization that was inherited from pg_stat_statements is removed,
shaving a good chunk of code.  On version upgrade, the past table is
removed, replaced by the new one.  Using the hint table requires an
upgrade up to 1.7.0.

Per pull request #193.
@michaelpq
Copy link
Collaborator Author

I got to discuss this patch with @yamatattsu in live last Friday, and he was OK with the change. I have now applied the patch to move on with the other release items on my plate.

@michaelpq michaelpq closed this May 20, 2024
@michaelpq michaelpq deleted the hint_table_query_id branch May 20, 2024 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant