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

Maskout script doesn't work on macOS #181

Closed
Sanath97 opened this issue Apr 10, 2024 · 3 comments
Closed

Maskout script doesn't work on macOS #181

Sanath97 opened this issue Apr 10, 2024 · 3 comments
Assignees

Comments

@Sanath97
Copy link

Maskout script used for tests maskout.sh doesn't seems to work on MAC machines. Particularly, issue is with sed command and script fails to mask cost/width values in the query plan. Instead we found below sed seems to work with both macOS and Linux machine(tested on Ubuntu).

sed -E 's/cost=10{7}[\.0-9]+ /cost={inf}..{inf} /;s/cost=[\.0-9]+ /cost=xxx..xxx /;s/width=[0-9]+([^0-9])/width=xxx\1/;s/^ *QUERY PLAN *$/ QUERY PLAN/;s/^--*$/----------------/'

@Sanath97 Sanath97 changed the title Maskout script doesn't work on MAC machine Maskout script doesn't work on macOS Apr 10, 2024
@michaelpq
Copy link
Collaborator

Yes, I want to replace the dependency to these two maskout shell scripts to a SQL function able to filter out the output of EXPLAIN, as we do in PostgreSQL for the main regression test suite. I'll see about doing this cleanup in the next few days for the upcoming release.

@michaelpq
Copy link
Collaborator

What I am referring here is to have an equivalent of explain_filter() in src/test/regress/sql/explain.sql, just that we'd define it in the init script here.

@michaelpq michaelpq self-assigned this Jul 22, 2024
sharmay added a commit to sharmay/pg_hint_plan that referenced this issue Aug 6, 2024
Obsolete maskout.sh and maskout2.sh
Use tuple only where required
michaelpq added a commit that referenced this issue 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 issue 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

The shell scripts have been removed from master and PG17 as of 9fad1e3, meaning that this issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants