Skip to content

Commit

Permalink
Remove pg_hint_plan.hints_anywhere
Browse files Browse the repository at this point in the history
This GUC setting was a wart, able to bypass a check to force hints to
happen before and/or after a set of characters.  A weird behavior that
it caused is that it was possible to define a hint within what should be
considered a value in a query.  For example, a query like that would use
a hint:
SELECT * FROM t1 JOIN t2 ON t1.val = t2.val
  WHERE '/*+MergeJoin(t1 t2)*/' <> '';

The reason why this parameter existed to begin with was to be more
liberal with the position of a hint, because pg_hint_plan has always
used a custom parser for query lookups.  An upcoming patch will be able
to solve these problems by introducing a proper query parser in this
extension, based on what upstream uses for psql with yyac.

Note that this GUC was not documented, as well, remaining hidden to the
end-user.
  • Loading branch information
michaelpq committed Oct 20, 2023
1 parent ef4fa9a commit b1f2aab
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 147 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ MODULES = pg_hint_plan
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 hints_anywhere plpgsql oldextversions
ut-fdw ut-W ut-T ut-fini plpgsql oldextversions
REGRESS_OPTS = --encoding=UTF8

EXTENSION = pg_hint_plan
Expand Down
83 changes: 0 additions & 83 deletions expected/hints_anywhere.out

This file was deleted.

64 changes: 25 additions & 39 deletions pg_hint_plan.c
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,6 @@ static int pg_hint_plan_parse_message_level = INFO;
static int pg_hint_plan_debug_message_level = LOG;
/* Default is off, to keep backward compatibility. */
static bool pg_hint_plan_enable_hint_table = false;
static bool pg_hint_plan_hints_anywhere = false;

static int plpgsql_recurse_level = 0; /* PLpgSQL recursion level */
static int recurse_level = 0; /* recursion level incl. direct SPI calls */
Expand Down Expand Up @@ -773,17 +772,6 @@ _PG_init(void)
assign_enable_hint_table,
NULL);

DefineCustomBoolVariable("pg_hint_plan.hints_anywhere",
"Read hints from anywhere in a query.",
"This option lets pg_hint_plan ignore syntax so be cautious for false reads.",
&pg_hint_plan_hints_anywhere,
false,
PGC_USERSET,
0,
NULL,
NULL,
NULL);

EmitWarningsOnPlaceholders("pg_hint_plan");

/* Install hooks. */
Expand Down Expand Up @@ -1973,34 +1961,32 @@ get_hints_from_comment(const char *p)
hint_head = strstr(p, HINT_START);
if (hint_head == NULL)
return NULL;
if (!pg_hint_plan_hints_anywhere)

for (;p < hint_head; p++)
{
for (;p < hint_head; p++)
{
/*
* Allow these characters precedes hint comment:
* - digits
* - alphabets which are in ASCII range
* - space, tabs and new-lines
* - underscores, for identifier
* - commas, for SELECT clause, EXPLAIN and PREPARE
* - parentheses, for EXPLAIN and PREPARE
* - squared brackets, for arrays (like arguments of PREPARE
* statements).
*
* Note that we don't use isalpha() nor isalnum() in ctype.h here to
* avoid behavior which depends on locale setting.
*/
if (!(*p >= '0' && *p <= '9') &&
!(*p >= 'A' && *p <= 'Z') &&
!(*p >= 'a' && *p <= 'z') &&
!isspace(*p) &&
*p != '_' &&
*p != ',' &&
*p != '(' && *p != ')' &&
*p != '[' && *p != ']')
return NULL;
}
/*
* Allow these characters precedes hint comment:
* - digits
* - alphabets which are in ASCII range
* - space, tabs and new-lines
* - underscores, for identifier
* - commas, for SELECT clause, EXPLAIN and PREPARE
* - parentheses, for EXPLAIN and PREPARE
* - squared brackets, for arrays (like arguments of PREPARE
* statements).
*
* Note that we don't use isalpha() nor isalnum() in ctype.h here to
* avoid behavior which depends on locale setting.
*/
if (!(*p >= '0' && *p <= '9') &&
!(*p >= 'A' && *p <= 'Z') &&
!(*p >= 'a' && *p <= 'z') &&
!isspace(*p) &&
*p != '_' &&
*p != ',' &&
*p != '(' && *p != ')' &&
*p != '[' && *p != ']')
return NULL;
}

head = (char *)hint_head;
Expand Down
24 changes: 0 additions & 24 deletions sql/hints_anywhere.sql

This file was deleted.

0 comments on commit b1f2aab

Please sign in to comment.