Re: pgsql: Add tests for reinit.c
On Sat, Mar 17, 2018 at 12:06 AM, David Steele wrote: > On 3/15/18 11:51 PM, Tom Lane wrote: > > I wrote: > >> Peter Eisentraut writes: > >>> Add tests for reinit.c > > > >> BTW, buildfarm member jacana hasn't succeeded at this test once. > >> The failures look like > > > >> ok 1 - init fork in base exists > >> ok 2 - main fork in base exists > >> error running SQL: 'psql::1: ERROR: directory > "/home/pgrunner/bf/root/HEAD/pgsql.build/src/test/recovery/tmp_check/tmp_test_wBGG" > does not exist' > >> while running 'psql -XAtq -d port=50531 host=127.0.0.1 > dbname='postgres' -f - -v ON_ERROR_STOP=1' with sql 'CREATE TABLESPACE ts1 > LOCATION > '/home/pgrunner/bf/root/HEAD/pgsql.build/src/test/recovery/tmp_check/tmp_test_wBGG'' > at > /home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/src/test/perl//PostgresNode.pm > line 1246. > > > >> so I hypothesize that there's something wrong with TestLib::tempdir on > >> Windows, but no idea what. > > > > After further staring at that, I think it's some sort of mingw path > > weirdness. I notice that the initdb output refers to the PGDATA > > directory as > > > > c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql. > build/src/test/recovery/tmp_check/t_014_unlogged_reinit_main_data/pgdata > > > > and then after that we see "pg_ctl start" succeeding with just > > > > /home/pgrunner/bf/root/HEAD/pgsql.build/src/test/recovery/ > tmp_check/t_014_unlogged_reinit_main_data/pgdata > > > > but this isn't working: > > > > /home/pgrunner/bf/root/HEAD/pgsql.build/src/test/recovery/ > tmp_check/tmp_test_esA3 > > > > perhaps because it lacks the "c:/mingw/msys/1.0" prefix. > > > > Still no opinion about how to fix it. > > I looked around for other examples but the only ones I could find are in > the pg_basebackup tests (010_pg_basebackup.pl), which exclude these > calls for Windows. I don't have a Windows machine to experiment on. > > Michael, any thoughts? > > The attached fixes it. We should probably put the function or something like it in TestLib.pm, though. The call to pwd is quite tricky - it needs to be done pretty much exactly like this, not quite sure why, but direct calls to "pwd -W" via system() or backticks don't work, only this indirect call works on jacana. cheers andrew tap-test-fix.patch Description: Binary data
pgsql: Fix typo in comment
Fix typo in comment Author: Daniel Gustafsson Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/71cce90ee99098f52e65278b96662e32ca005771 Modified Files -- src/backend/libpq/be-secure-openssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: pgsql: Add tests for reinit.c
On 3/19/18 4:15 AM, Andrew Dunstan wrote: > > The attached fixes it. We should probably put the function or something > like it in TestLib.pm, though. The call to pwd is quite tricky - it > needs to be done pretty much exactly like this, not quite sure why, but > direct calls to "pwd -W" via system() or backticks don't work, only this > indirect call works on jacana. Thanks for the patch, Andrew! Peter, would you like me to provide a patch that moves this function to TestLib.pm? Happy to do it, just don't want patches crossing in the night, as it were. Thanks, -- -David da...@pgmasters.net
pgsql: Rewrite recurse_union_children to iterate, rather than recurse.
Rewrite recurse_union_children to iterate, rather than recurse. Also, rename it to plan_union_chidren, so the old name wasn't very descriptive. This results in a small net reduction in code, seems at least to me to be easier to understand, and saves space on the process stack. Patch by me, reviewed and tested by Ashutosh Bapat and Rajkumar Raghuwanshi. Discussion: http://postgr.es/m/CA+TgmoaLRAOqHmMZx=esm3vdepceg+-xxzsrxq8gtfjo_zb...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/49525c46309828b3024fe8040fa99c7dcc83933d Modified Files -- src/backend/optimizer/prep/prepunion.c | 100 - 1 file changed, 47 insertions(+), 53 deletions(-)
pgsql: Generate a separate upper relation for each stage of setop plann
Generate a separate upper relation for each stage of setop planning. Commit 3fc6e2d7f5b652b417fa6937c34de2438d60fa9f made setop planning stages return paths rather than plans, but all such paths were loosely associated with a single RelOptInfo, and only the final path was added to the RelOptInfo. Even at the time, it was foreseen that this should be changed, because there is otherwise no good way for a single stage of setop planning to return multiple paths. With this patch, each stage of set operation planning now creates a separate RelOptInfo; these are distinguished by using appropriate relid sets. Note that this patch does nothing whatsoever about actually returning multiple paths for the same set operation; it just makes it possible for a future patch to do so. Along the way, adjust things so that create_upper_paths_hook is called for each of these new RelOptInfos rather than just once, since that might be useful to extensions using that hook. It might be a good to provide an FDW API here as well, but I didn't try to do that for now. Patch by me, reviewed and tested by Ashutosh Bapat and Rajkumar Raghuwanshi. Discussion: http://postgr.es/m/CA+TgmoaLRAOqHmMZx=esm3vdepceg+-xxzsrxq8gtfjo_zb...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c596fadbfe20ff50a8e5f4bc4b4ff5b7c302ecc0 Modified Files -- src/backend/optimizer/prep/prepunion.c | 340 ++--- 1 file changed, 190 insertions(+), 150 deletions(-)
pgsql: Fix state reversal after partition tuple routing
Fix state reversal after partition tuple routing We make some changes to ModifyTableState and the EState it uses whenever we route tuples to partitions; but we weren't restoring properly in all cases, possibly causing crashes when partitions with different tuple descriptors are targeted by tuples inserted in the same command. Refactor some code, creating ExecPrepareTupleRouting, to encapsulate the needed state changing logic, and have it invoked one level above its current place (ie. put it in ExecModifyTable instead of ExecInsert); this makes it all more readable. Add a test case to exercise this. We don't support having views as partitions; and since only views can have INSTEAD OF triggers, there is no point in testing for INSTEAD OF when processing insertions into a partitioned table. Remove code that appears to support this (but which is actually never relevant.) In passing, fix location of some very confusing comments in ModifyTableState. Reported-by: Amit Langote Author: Etsuro Fujita, Amit Langote Discussion: https://postgr/es/m/0473bf5c-57b1-f1f7-3d58-455c2230b...@lab.ntt.co.jp Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/e3faddf537903144553abd989b156168cb7984df Modified Files -- src/backend/commands/copy.c| 20 +-- src/backend/executor/nodeModifyTable.c | 216 ++--- src/include/nodes/execnodes.h | 17 ++- src/test/regress/expected/insert.out | 26 src/test/regress/sql/insert.sql| 23 5 files changed, 189 insertions(+), 113 deletions(-)
pgsql: Fix state reversal after partition tuple routing
Fix state reversal after partition tuple routing We make some changes to ModifyTableState and the EState it uses whenever we route tuples to partitions; but we weren't restoring properly in all cases, possibly causing crashes when partitions with different tuple descriptors are targeted by tuples inserted in the same command. Refactor some code, creating ExecPrepareTupleRouting, to encapsulate the needed state changing logic, and have it invoked one level above its current place (ie. put it in ExecModifyTable instead of ExecInsert); this makes it all more readable. Add a test case to exercise this. We don't support having views as partitions; and since only views can have INSTEAD OF triggers, there is no point in testing for INSTEAD OF when processing insertions into a partitioned table. Remove code that appears to support this (but which is actually never relevant.) In passing, fix location of some very confusing comments in ModifyTableState. Reported-by: Amit Langote Author: Etsuro Fujita, Amit Langote Discussion: https://postgr/es/m/0473bf5c-57b1-f1f7-3d58-455c2230b...@lab.ntt.co.jp Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ee49f49c4a6b008591aea457becffa0df041 Modified Files -- src/backend/commands/copy.c| 20 +-- src/backend/executor/nodeModifyTable.c | 230 + src/include/nodes/execnodes.h | 12 +- src/test/regress/expected/insert.out | 26 src/test/regress/sql/insert.sql| 23 5 files changed, 188 insertions(+), 123 deletions(-)
pgsql: Expand comment a little bit
Expand comment a little bit The previous commit removed a comment that was a bit more verbose than its replacement. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/839a8eb2b3df68e105fb4f7a72e71652d6becc7a Modified Files -- src/backend/executor/nodeModifyTable.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-)
pgsql: Remove unnecessary members from ModifyTableState and ExecInsert
Remove unnecessary members from ModifyTableState and ExecInsert These values can be obtained from the ModifyTable node which is already a part of both the ModifyTableState and ExecInsert. Author: Álvaro Herrera, Amit Langote Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/20180316151303.rml2p5wffn3o6qy6@alvherre.pgsql Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ee0a1fc84eb29c916687dc5bd26909401d3aa8cd Modified Files -- src/backend/executor/execPartition.c | 4 ++-- src/backend/executor/nodeModifyTable.c | 34 +++--- src/include/nodes/execnodes.h | 3 --- 3 files changed, 21 insertions(+), 20 deletions(-)
pgsql: Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY
Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY. Jeff Janes discovered that commit 7ca25b7de made one of the queries run by REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly. The root cause is bad cardinality estimation for correlated quals, but a principled solution to that problem is some way off, especially since the planner lacks any statistics about whole-row variables. Moreover, in non-error cases this query produces no rows, meaning it must be run to completion; but use of LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan, exactly not what we want. Remove the LIMIT clause, and instead rely on the count parameter we pass to SPI_execute() to prevent excess work if the query does return some rows. While we've heard no field reports of planner misbehavior with this query, it could be that people are having performance issues that haven't reached the level of pain needed to cause a bug report. In any case, that LIMIT clause can't possibly do anything helpful with any existing version of the planner, and it demonstrably can cause bad choices in some cases, so back-patch to 9.4 where the code was introduced. Thomas Munro Discussion: https://postgr.es/m/CAMkU=1z-jogymhneghar1cru4f1xdfhqjdzxp_ctk5cl3do...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6fbd5cce22ebd2203d99cd7dcd179d0e1138599e Modified Files -- src/backend/commands/matview.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY
Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY. Jeff Janes discovered that commit 7ca25b7de made one of the queries run by REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly. The root cause is bad cardinality estimation for correlated quals, but a principled solution to that problem is some way off, especially since the planner lacks any statistics about whole-row variables. Moreover, in non-error cases this query produces no rows, meaning it must be run to completion; but use of LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan, exactly not what we want. Remove the LIMIT clause, and instead rely on the count parameter we pass to SPI_execute() to prevent excess work if the query does return some rows. While we've heard no field reports of planner misbehavior with this query, it could be that people are having performance issues that haven't reached the level of pain needed to cause a bug report. In any case, that LIMIT clause can't possibly do anything helpful with any existing version of the planner, and it demonstrably can cause bad choices in some cases, so back-patch to 9.4 where the code was introduced. Thomas Munro Discussion: https://postgr.es/m/CAMkU=1z-jogymhneghar1cru4f1xdfhqjdzxp_ctk5cl3do...@mail.gmail.com Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b6ba94ec45bbf49a95a31c4ef740d1d60de7b143 Modified Files -- src/backend/commands/matview.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY
Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY. Jeff Janes discovered that commit 7ca25b7de made one of the queries run by REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly. The root cause is bad cardinality estimation for correlated quals, but a principled solution to that problem is some way off, especially since the planner lacks any statistics about whole-row variables. Moreover, in non-error cases this query produces no rows, meaning it must be run to completion; but use of LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan, exactly not what we want. Remove the LIMIT clause, and instead rely on the count parameter we pass to SPI_execute() to prevent excess work if the query does return some rows. While we've heard no field reports of planner misbehavior with this query, it could be that people are having performance issues that haven't reached the level of pain needed to cause a bug report. In any case, that LIMIT clause can't possibly do anything helpful with any existing version of the planner, and it demonstrably can cause bad choices in some cases, so back-patch to 9.4 where the code was introduced. Thomas Munro Discussion: https://postgr.es/m/CAMkU=1z-jogymhneghar1cru4f1xdfhqjdzxp_ctk5cl3do...@mail.gmail.com Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c553e4a5074383b7e234907ccc70bde9b098ef1a Modified Files -- src/backend/commands/matview.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY
Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY. Jeff Janes discovered that commit 7ca25b7de made one of the queries run by REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly. The root cause is bad cardinality estimation for correlated quals, but a principled solution to that problem is some way off, especially since the planner lacks any statistics about whole-row variables. Moreover, in non-error cases this query produces no rows, meaning it must be run to completion; but use of LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan, exactly not what we want. Remove the LIMIT clause, and instead rely on the count parameter we pass to SPI_execute() to prevent excess work if the query does return some rows. While we've heard no field reports of planner misbehavior with this query, it could be that people are having performance issues that haven't reached the level of pain needed to cause a bug report. In any case, that LIMIT clause can't possibly do anything helpful with any existing version of the planner, and it demonstrably can cause bad choices in some cases, so back-patch to 9.4 where the code was introduced. Thomas Munro Discussion: https://postgr.es/m/CAMkU=1z-jogymhneghar1cru4f1xdfhqjdzxp_ctk5cl3do...@mail.gmail.com Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ebcf34d4628e175a7f35714ccec561b596fe6d0f Modified Files -- src/backend/commands/matview.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY
Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY. Jeff Janes discovered that commit 7ca25b7de made one of the queries run by REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly. The root cause is bad cardinality estimation for correlated quals, but a principled solution to that problem is some way off, especially since the planner lacks any statistics about whole-row variables. Moreover, in non-error cases this query produces no rows, meaning it must be run to completion; but use of LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan, exactly not what we want. Remove the LIMIT clause, and instead rely on the count parameter we pass to SPI_execute() to prevent excess work if the query does return some rows. While we've heard no field reports of planner misbehavior with this query, it could be that people are having performance issues that haven't reached the level of pain needed to cause a bug report. In any case, that LIMIT clause can't possibly do anything helpful with any existing version of the planner, and it demonstrably can cause bad choices in some cases, so back-patch to 9.4 where the code was introduced. Thomas Munro Discussion: https://postgr.es/m/CAMkU=1z-jogymhneghar1cru4f1xdfhqjdzxp_ctk5cl3do...@mail.gmail.com Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1568156d8fe1983756ac747bd5f895e5ef6a66fa Modified Files -- src/backend/commands/matview.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Don't use an Msys virtual path to create a tablespace
Don't use an Msys virtual path to create a tablespace The new unlogged_reinit recovery tests create a new tablespace using TestLib.pm's tempdir. However, on msys that function returns a virtual path that isn't understood by Postgres. Here we add a new function to TestLib.pm to turn such a path into a real path on the underlying file system, and use it in the new test to create the tablespace. The new function is essentially a NOOP everywhere but msys. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9ad21a6957ff2d8743e9a59ba062d3c009b24ec4 Modified Files -- src/test/perl/TestLib.pm | 18 ++ src/test/recovery/t/014_unlogged_reinit.pl | 4 +++- 2 files changed, 21 insertions(+), 1 deletion(-)
Re: pgsql: Add tests for reinit.c
On Mon, Mar 19, 2018 at 11:33 PM, David Steele wrote: > On 3/19/18 4:15 AM, Andrew Dunstan wrote: >> >> The attached fixes it. We should probably put the function or something >> like it in TestLib.pm, though. The call to pwd is quite tricky - it >> needs to be done pretty much exactly like this, not quite sure why, but >> direct calls to "pwd -W" via system() or backticks don't work, only this >> indirect call works on jacana. > > Thanks for the patch, Andrew! > > Peter, would you like me to provide a patch that moves this function to > TestLib.pm? Happy to do it, just don't want patches crossing in the > night, as it were. > I've reworked it and pushed the fix. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pgsql: Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURR
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY. refresh_by_match_merge() has some issues in the way it builds a SQL query to construct the "diff" table: 1. It doesn't require the selected unique index(es) to be indimmediate. 2. It doesn't pay attention to the particular equality semantics enforced by a given index, but just assumes that they must be those of the column datatype's default btree opclass. 3. It doesn't check that the indexes are btrees. 4. It's insufficiently careful to ensure that the parser will pick the intended operator when parsing the query. (This would have been a security bug before CVE-2018-1058.) 5. It's not careful about indexes on system columns. The way to fix #4 is to make use of the existing code in ri_triggers.c for generating an arbitrary binary operator clause. I chose to move that to ruleutils.c, since that seems a more reasonable place to be exporting such functionality from than ri_triggers.c. While #1, #3, and #5 are just latent given existing feature restrictions, and #2 doesn't arise in the core system for lack of alternate opclasses with different equality behaviors, #4 seems like an issue worth back-patching. That's the bulk of the change anyway, so just back-patch the whole thing to 9.4 where this code was introduced. Discussion: https://postgr.es/m/13836.1521413...@sss.pgh.pa.us Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/e17e9055f5644f1b39ecd1bf64ec03d3430dfb46 Modified Files -- src/backend/commands/matview.c | 182 +++- src/backend/utils/adt/ri_triggers.c | 69 ++ src/backend/utils/adt/ruleutils.c | 80 src/include/utils/builtins.h| 4 + 4 files changed, 208 insertions(+), 127 deletions(-)
pgsql: Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURR
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY. refresh_by_match_merge() has some issues in the way it builds a SQL query to construct the "diff" table: 1. It doesn't require the selected unique index(es) to be indimmediate. 2. It doesn't pay attention to the particular equality semantics enforced by a given index, but just assumes that they must be those of the column datatype's default btree opclass. 3. It doesn't check that the indexes are btrees. 4. It's insufficiently careful to ensure that the parser will pick the intended operator when parsing the query. (This would have been a security bug before CVE-2018-1058.) 5. It's not careful about indexes on system columns. The way to fix #4 is to make use of the existing code in ri_triggers.c for generating an arbitrary binary operator clause. I chose to move that to ruleutils.c, since that seems a more reasonable place to be exporting such functionality from than ri_triggers.c. While #1, #3, and #5 are just latent given existing feature restrictions, and #2 doesn't arise in the core system for lack of alternate opclasses with different equality behaviors, #4 seems like an issue worth back-patching. That's the bulk of the change anyway, so just back-patch the whole thing to 9.4 where this code was introduced. Discussion: https://postgr.es/m/13836.1521413...@sss.pgh.pa.us Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/aa1cacd9541a4b46b3ca416d987e3b4a681ac5f3 Modified Files -- src/backend/commands/matview.c | 161 +--- src/backend/utils/adt/ri_triggers.c | 69 ++-- src/backend/utils/adt/ruleutils.c | 80 ++ src/include/utils/builtins.h| 4 + 4 files changed, 203 insertions(+), 111 deletions(-)
pgsql: Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURR
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY. refresh_by_match_merge() has some issues in the way it builds a SQL query to construct the "diff" table: 1. It doesn't require the selected unique index(es) to be indimmediate. 2. It doesn't pay attention to the particular equality semantics enforced by a given index, but just assumes that they must be those of the column datatype's default btree opclass. 3. It doesn't check that the indexes are btrees. 4. It's insufficiently careful to ensure that the parser will pick the intended operator when parsing the query. (This would have been a security bug before CVE-2018-1058.) 5. It's not careful about indexes on system columns. The way to fix #4 is to make use of the existing code in ri_triggers.c for generating an arbitrary binary operator clause. I chose to move that to ruleutils.c, since that seems a more reasonable place to be exporting such functionality from than ri_triggers.c. While #1, #3, and #5 are just latent given existing feature restrictions, and #2 doesn't arise in the core system for lack of alternate opclasses with different equality behaviors, #4 seems like an issue worth back-patching. That's the bulk of the change anyway, so just back-patch the whole thing to 9.4 where this code was introduced. Discussion: https://postgr.es/m/13836.1521413...@sss.pgh.pa.us Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7d7b59aa65918f59e3c122318b31e0f77f12f2cd Modified Files -- src/backend/commands/matview.c | 182 +++- src/backend/utils/adt/ri_triggers.c | 69 ++ src/backend/utils/adt/ruleutils.c | 80 src/include/utils/builtins.h| 4 + 4 files changed, 208 insertions(+), 127 deletions(-)
pgsql: Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURR
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY. refresh_by_match_merge() has some issues in the way it builds a SQL query to construct the "diff" table: 1. It doesn't require the selected unique index(es) to be indimmediate. 2. It doesn't pay attention to the particular equality semantics enforced by a given index, but just assumes that they must be those of the column datatype's default btree opclass. 3. It doesn't check that the indexes are btrees. 4. It's insufficiently careful to ensure that the parser will pick the intended operator when parsing the query. (This would have been a security bug before CVE-2018-1058.) 5. It's not careful about indexes on system columns. The way to fix #4 is to make use of the existing code in ri_triggers.c for generating an arbitrary binary operator clause. I chose to move that to ruleutils.c, since that seems a more reasonable place to be exporting such functionality from than ri_triggers.c. While #1, #3, and #5 are just latent given existing feature restrictions, and #2 doesn't arise in the core system for lack of alternate opclasses with different equality behaviors, #4 seems like an issue worth back-patching. That's the bulk of the change anyway, so just back-patch the whole thing to 9.4 where this code was introduced. Discussion: https://postgr.es/m/13836.1521413...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6497a18e6c1b5874566a77737ec3d381fded3ec2 Modified Files -- src/backend/commands/matview.c | 181 +++- src/backend/utils/adt/ri_triggers.c | 69 ++ src/backend/utils/adt/ruleutils.c | 80 src/include/utils/builtins.h| 4 + 4 files changed, 207 insertions(+), 127 deletions(-)
pgsql: Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURR
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY. refresh_by_match_merge() has some issues in the way it builds a SQL query to construct the "diff" table: 1. It doesn't require the selected unique index(es) to be indimmediate. 2. It doesn't pay attention to the particular equality semantics enforced by a given index, but just assumes that they must be those of the column datatype's default btree opclass. 3. It doesn't check that the indexes are btrees. 4. It's insufficiently careful to ensure that the parser will pick the intended operator when parsing the query. (This would have been a security bug before CVE-2018-1058.) 5. It's not careful about indexes on system columns. The way to fix #4 is to make use of the existing code in ri_triggers.c for generating an arbitrary binary operator clause. I chose to move that to ruleutils.c, since that seems a more reasonable place to be exporting such functionality from than ri_triggers.c. While #1, #3, and #5 are just latent given existing feature restrictions, and #2 doesn't arise in the core system for lack of alternate opclasses with different equality behaviors, #4 seems like an issue worth back-patching. That's the bulk of the change anyway, so just back-patch the whole thing to 9.4 where this code was introduced. Discussion: https://postgr.es/m/13836.1521413...@sss.pgh.pa.us Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/e1f186da949dff96f974f491c0248b44845cc3e8 Modified Files -- src/backend/commands/matview.c | 161 +--- src/backend/utils/adt/ri_triggers.c | 69 ++-- src/backend/utils/adt/ruleutils.c | 80 ++ src/include/utils/builtins.h| 4 + 4 files changed, 203 insertions(+), 111 deletions(-)
pgsql: Add missing break
Add missing break Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/13c7c65ec900a30bcddcb27f5fd138dcdbc2ca2e Modified Files -- src/backend/parser/analyze.c | 1 + 1 file changed, 1 insertion(+)
Re: pgsql: Set libpq sslcompression to off by default
On 3/17/18 15:12, Daniel Gustafsson wrote: >> On 17 Mar 2018, at 17:47, Tom Lane wrote: >> >> Peter Eisentraut writes: >>> Set libpq sslcompression to off by default >> >> Buildfarm reports that SSL_clear_options isn't available everywhere. > > Per some reading of the documentation and various patchers it seems > SSL_clear_options() was introduced in 0.9.8m and SSL_OP_NO_COMPRESSION in > 1.0.0. It seems the failure is limited to an old NetBSD version. They might have patched their libssl locally somehow. Is it worth supporting this? We could add a configure test for SSL_clear_options(). -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgsql: Add tests for reinit.c
On 3/19/18 5:58 PM, Andrew Dunstan wrote: > On Mon, Mar 19, 2018 at 11:33 PM, David Steele wrote: >> On 3/19/18 4:15 AM, Andrew Dunstan wrote: >>> >>> The attached fixes it. We should probably put the function or something >>> like it in TestLib.pm, though. The call to pwd is quite tricky - it >>> needs to be done pretty much exactly like this, not quite sure why, but >>> direct calls to "pwd -W" via system() or backticks don't work, only this >>> indirect call works on jacana. >> >> Thanks for the patch, Andrew! >> >> Peter, would you like me to provide a patch that moves this function to >> TestLib.pm? Happy to do it, just don't want patches crossing in the >> night, as it were. >> > > I've reworked it and pushed the fix. Thanks, Andrew! -- -David da...@pgmasters.net
pgsql: Prevent query-lifespan memory leakage of SP-GiST traversal value
Prevent query-lifespan memory leakage of SP-GiST traversal values. The original coding of the SP-GiST scan traversalValue feature (commit ccd6eb49a) arranged for traversal values to be stored in the query's main executor context. That's fine if there's only one index scan per query, but if there are many, we have a memory leak as successive scans create new traversal values. Fix it by creating a separate memory context for traversal values, which we can reset during spgrescan(). Back-patch to 9.6 where this code was introduced. In principle, adding the traversalCxt field to SpGistScanOpaqueData creates an ABI break in the back branches. But I (tgl) have little sympathy for extensions including spgist_private.h, so I'm not very worried about that. Alternatively we could stick the new field at the end of the struct in back branches, but that has its own downsides. Anton Dignös, reviewed by Alexander Kuzmenkov Discussion: https://postgr.es/m/calndv1jb6y2te-m8xhlxlx12rsbmzj1f4hesx7j0hjgyoha...@mail.gmail.com Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/57ef2da434e235f6295bf49a065fce3da398fd8b Modified Files -- src/backend/access/spgist/spgscan.c | 9 - src/include/access/spgist_private.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
pgsql: Prevent query-lifespan memory leakage of SP-GiST traversal value
Prevent query-lifespan memory leakage of SP-GiST traversal values. The original coding of the SP-GiST scan traversalValue feature (commit ccd6eb49a) arranged for traversal values to be stored in the query's main executor context. That's fine if there's only one index scan per query, but if there are many, we have a memory leak as successive scans create new traversal values. Fix it by creating a separate memory context for traversal values, which we can reset during spgrescan(). Back-patch to 9.6 where this code was introduced. In principle, adding the traversalCxt field to SpGistScanOpaqueData creates an ABI break in the back branches. But I (tgl) have little sympathy for extensions including spgist_private.h, so I'm not very worried about that. Alternatively we could stick the new field at the end of the struct in back branches, but that has its own downsides. Anton Dignös, reviewed by Alexander Kuzmenkov Discussion: https://postgr.es/m/calndv1jb6y2te-m8xhlxlx12rsbmzj1f4hesx7j0hjgyoha...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/467963c3e9c5ba9a953959f8aebcdd7206188a18 Modified Files -- src/backend/access/spgist/spgscan.c | 9 - src/include/access/spgist_private.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
pgsql: Prevent query-lifespan memory leakage of SP-GiST traversal value
Prevent query-lifespan memory leakage of SP-GiST traversal values. The original coding of the SP-GiST scan traversalValue feature (commit ccd6eb49a) arranged for traversal values to be stored in the query's main executor context. That's fine if there's only one index scan per query, but if there are many, we have a memory leak as successive scans create new traversal values. Fix it by creating a separate memory context for traversal values, which we can reset during spgrescan(). Back-patch to 9.6 where this code was introduced. In principle, adding the traversalCxt field to SpGistScanOpaqueData creates an ABI break in the back branches. But I (tgl) have little sympathy for extensions including spgist_private.h, so I'm not very worried about that. Alternatively we could stick the new field at the end of the struct in back branches, but that has its own downsides. Anton Dignös, reviewed by Alexander Kuzmenkov Discussion: https://postgr.es/m/calndv1jb6y2te-m8xhlxlx12rsbmzj1f4hesx7j0hjgyoha...@mail.gmail.com Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d18a88acf2d1591f9332bdb1da918037c90d3aa0 Modified Files -- src/backend/access/spgist/spgscan.c | 9 - src/include/access/spgist_private.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
Re: pgsql: Set libpq sslcompression to off by default
Peter Eisentraut writes: > On 3/17/18 15:12, Daniel Gustafsson wrote: >> On 17 Mar 2018, at 17:47, Tom Lane wrote: >>> Buildfarm reports that SSL_clear_options isn't available everywhere. >> Per some reading of the documentation and various patchers it seems >> SSL_clear_options() was introduced in 0.9.8m and SSL_OP_NO_COMPRESSION in >> 1.0.0. > It seems the failure is limited to an old NetBSD version. They might > have patched their libssl locally somehow. Is it worth supporting this? Dunno, but the other side of the coin is that the goals of this patch don't seem like a sufficient reason to break backwards compatibility with any platform. > We could add a configure test for SSL_clear_options(). Kind of annoying, but ... regards, tom lane
