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

Fix issue #181 #198

Closed
wants to merge 1 commit into from
Closed

Fix issue #181 #198

wants to merge 1 commit into from

Conversation

sharmay
Copy link

@sharmay sharmay commented Aug 6, 2024

Obsolete maskout.sh and maskout2.sh and use PostgreSQL function based on regexp_replace.
Use tuple only where required
Updated expected to

  • remove tuple header and row summary
  • Add new function

Obsolete maskout.sh and maskout2.sh
Use tuple only where required
michaelpq added a commit that referenced this pull request Aug 20, 2024
pg_hint_plan has depended for a long time on a set of non-portable shell
scripts to filter the output of the plans of any unstable output, like
costs or widths.  This had the disadvantage to be usable only on Linux,
while depending on \o and temporary output files.

This is replaced in this commit by a solution closer to PostgreSQL
upstream, where we use a PL/pgSQL function to process the EXPLAIN
queries whose output need to be stabilized.  The style used in this
commit may arguably be improved more in the future, but the changes done
here make the diffs more pallatable than anything I have considered,
with all the plans generated remaining the same.

Some queries that included quotes in ut-R required a couple more quotes
to work in the filtering function.  Some extra CONTEXT messages coming
from the filtering function are generated, as well as some extra LOG
messages for cases related unused indexes, but let's live with that for
now.

Author: Yogesh Sharma, Michael Paquier
Backpatch-through: 17

Per pull request #198 and issue #181.
michaelpq added a commit that referenced this pull request Aug 20, 2024
pg_hint_plan has depended for a long time on a set of non-portable shell
scripts to filter the output of the plans of any unstable output, like
costs or widths.  This had the disadvantage to be usable only on Linux,
while depending on \o and temporary output files.

This is replaced in this commit by a solution closer to PostgreSQL
upstream, where we use a PL/pgSQL function to process the EXPLAIN
queries whose output need to be stabilized.  The style used in this
commit may arguably be improved more in the future, but the changes done
here make the diffs more pallatable than anything I have considered,
with all the plans generated remaining the same.

Some queries that included quotes in ut-R required a couple more quotes
to work in the filtering function.  Some extra CONTEXT messages coming
from the filtering function are generated, as well as some extra LOG
messages for cases related unused indexes, but let's live with that for
now.

Author: Yogesh Sharma, Michael Paquier
Backpatch-through: 17

Per pull request #198 and issue #181.
@michaelpq
Copy link
Collaborator

There have been quite a few things I have been annoyed with here:

  • The dependency to cat to read the temporary output files. This did not move far enough the needle in terms of portability.
  • The extra tweaks for QUERY_PLAN, which were somewhat required because each script had a slight different way of applying its filtering. I've unified the whole with a unique function, even if it adds a few lines of output.
  • The temporary file generation had better be removed, so I have switched to a function like Postgres, with an EXECUTE on which the filtering is applied. On ERROR, this adds a few CONTEXT messages from the function, but I'm OK to live with that.
  • The indentation filtering was generating some extra diffs, making the plans harder to think through.

The changes were really mechanical, with ut-R requiring a few quotes for some queries, but I have prioritized the point that diffs should be a maximum edible, which is why I have applied a couple of extra newlines with the explain_filter() calls. It's easier to check the original output, with the original plans not changing.

@michaelpq michaelpq closed this Aug 20, 2024
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.

2 participants