Skip to content

Commit

Permalink
Handle better the hint table if the extension is not created
Browse files Browse the repository at this point in the history
An environment enabling pg_hint_plan.enable_hint_table while
pg_hint_plan has not been created as an extension would fail all its
queries as the table created by the extension does not exist.

This commit adds a check at the top of get_hints_from_table() to check
if the extension is installed before attempting to use the hint table,
generating a WARNING.  We have discussed about a few options, but a
WARNING is more consistent with how duplicated hints or compute_query_id
disabled are handled.

This does not completely close the failure hole, though, as it is still
possible to see the table lookup failure for the CREATE EXTENSION
command enabling pg_hint_plan as an extension if enable_hint_table is
enabled.  In terms of code simplicity, I am not really convinced that
we need to do more just for that.

This commit relies on 490f869d92e5 that has introduced syscache entries
for pg_extension.  On stable branches, we will need a different logic
with a check on the table itself with its namespace.

Idea based on some feedback from Julien Rouhaud.  The tests are from me.

Author: Sami Imseih, Michael Paquier

Backpatch-through: 12

Per issue #191.
  • Loading branch information
michaelpq committed Aug 22, 2024
1 parent 9849b00 commit 44f73c6
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ OBJS = \
HINTPLANVER = 1.7.0

REGRESS = init base_plan pg_hint_plan ut-init ut-A ut-S ut-J ut-L ut-G ut-R \
ut-fdw ut-W ut-T ut-fini plpgsql oldextversions
ut-fdw ut-W ut-T ut-fini plpgsql hint_table oldextversions
REGRESS_OPTS = --encoding=UTF8

EXTENSION = pg_hint_plan
Expand Down
24 changes: 24 additions & 0 deletions expected/hint_table.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
-- Tests for the hint table
LOAD 'pg_hint_plan';
-- Attempting to use the hint table without the extension created
-- emits a WARNING.
SET pg_hint_plan.enable_hint_table TO on;
SELECT 1;
WARNING: cannot use the hint table
HINT: Run "CREATE EXTENSION pg_hint_plan" to create the hint table.
?column?
----------
1
(1 row)

SET pg_hint_plan.enable_hint_table TO off;
CREATE EXTENSION pg_hint_plan;
SET pg_hint_plan.enable_hint_table TO on;
SELECT 1;
?column?
----------
1
(1 row)

SET pg_hint_plan.enable_hint_table TO off;
DROP EXTENSION pg_hint_plan;
20 changes: 20 additions & 0 deletions pg_hint_plan.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "access/genam.h"
#include "access/heapam.h"
#include "access/relation.h"
#include "catalog/namespace.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_index.h"
#include "catalog/pg_proc.h"
Expand Down Expand Up @@ -1856,6 +1857,25 @@ get_hints_from_table(uint64 queryId, const char *client_application)
Datum values[2];
char nulls[2] = {' ', ' '};
text *app;
Oid namespaceId;
bool hints_table_found = false;

/*
* Make sure that hint_plan.hints is found before we attempt to look for
* a hint.
*/
namespaceId = LookupExplicitNamespace("hint_plan", true);
if (OidIsValid(namespaceId) &&
OidIsValid(get_relname_relid("hints", namespaceId)))
hints_table_found = true;

if (!hints_table_found)
{
ereport(WARNING,
(errmsg ("cannot use the hint table"),
errhint("Run \"CREATE EXTENSION pg_hint_plan\" to create the hint table.")));
return NULL;
}

PG_TRY();
{
Expand Down
14 changes: 14 additions & 0 deletions sql/hint_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- Tests for the hint table
LOAD 'pg_hint_plan';

-- Attempting to use the hint table without the extension created
-- emits a WARNING.
SET pg_hint_plan.enable_hint_table TO on;
SELECT 1;
SET pg_hint_plan.enable_hint_table TO off;

CREATE EXTENSION pg_hint_plan;
SET pg_hint_plan.enable_hint_table TO on;
SELECT 1;
SET pg_hint_plan.enable_hint_table TO off;
DROP EXTENSION pg_hint_plan;

0 comments on commit 44f73c6

Please sign in to comment.