Skip to content

Commit 885fc50

Browse files
committed
Clean up get_pgsm_query_id_hash()
The function was unnecessarily convoluted so clean it up by making the parser easier to read, asserting against NULL input and wrapping the comment at the right width. Especially the code for handling whitespace was much more complicated than necessary. Also add another test example to check that we merge whitespace even when split by a comment.
1 parent a6eb046 commit 885fc50

5 files changed

Lines changed: 62 additions & 58 deletions

File tree

regression/expected/pgsm_query_id.out

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ SELECT *, add(1, 2) FROM t1;
3535
---+-----
3636
(0 rows)
3737

38+
SELECT * /* x */ FROM t2;
39+
b
40+
---
41+
(0 rows)
42+
3843
SELECT * FROM t2;
3944
b
4045
---
@@ -85,7 +90,7 @@ SELECT datname, pgsm_query_id, query, calls FROM pg_stat_monitor ORDER BY pgsm_q
8590
--------------------+---------------------+-----------------------------------------------------+-------
8691
contrib_regression | 689150021118383254 | SELECT pg_stat_monitor_reset() | 1
8792
db2 | 1648576050146230147 | SELECT * FROM t3 WHERE c = 20 | 1
88-
db1 | 1897482803466821995 | SELECT * FROM t2 | 3
93+
db1 | 1897482803466821995 | SELECT * /* x */ FROM t2 | 4
8994
db1 | 1988437669671417938 | SELECT * FROM t1 | 1
9095
db2 | 1988437669671417938 | SELECT * FROM t1 | 1
9196
db1 | 5193804135051352284 | SELECT $1 + $2 | 1

regression/expected/pgsm_query_id_1.out

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ SELECT *, add(1, 2) FROM t1;
3535
---+-----
3636
(0 rows)
3737

38+
SELECT * /* x */ FROM t2;
39+
b
40+
---
41+
(0 rows)
42+
3843
SELECT * FROM t2;
3944
b
4045
---
@@ -85,7 +90,7 @@ SELECT datname, pgsm_query_id, query, calls FROM pg_stat_monitor ORDER BY pgsm_q
8590
--------------------+---------------------+-----------------------------------------------------+-------
8691
contrib_regression | 689150021118383254 | SELECT pg_stat_monitor_reset() | 1
8792
db2 | 1648576050146230147 | SELECT * FROM t3 WHERE c = 20 | 1
88-
db1 | 1897482803466821995 | SELECT * FROM t2 | 3
93+
db1 | 1897482803466821995 | SELECT * /* x */ FROM t2 | 4
8994
db1 | 1988437669671417938 | SELECT * FROM t1 | 1
9095
db2 | 1988437669671417938 | SELECT * FROM t1 | 1
9196
db2 | 6220142855706866455 | set pg_stat_monitor.pgsm_enable_pgsm_query_id = on | 1

regression/expected/pgsm_query_id_2.out

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ SELECT *, add(1, 2) FROM t1;
3535
---+-----
3636
(0 rows)
3737

38+
SELECT * /* x */ FROM t2;
39+
b
40+
---
41+
(0 rows)
42+
3843
SELECT * FROM t2;
3944
b
4045
---
@@ -85,7 +90,7 @@ SELECT datname, pgsm_query_id, query, calls FROM pg_stat_monitor ORDER BY pgsm_q
8590
--------------------+---------------------+-----------------------------------------------------+-------
8691
contrib_regression | 689150021118383254 | SELECT pg_stat_monitor_reset() | 1
8792
db2 | 1648576050146230147 | SELECT * FROM t3 WHERE c = 20 | 1
88-
db1 | 1897482803466821995 | SELECT * FROM t2 | 3
93+
db1 | 1897482803466821995 | SELECT * /* x */ FROM t2 | 4
8994
db1 | 1988437669671417938 | SELECT * FROM t1 | 1
9095
db2 | 1988437669671417938 | SELECT * FROM t1 | 1
9196
db1 | 8828613017640075704 | SELECT *, add(1, 2) FROM t1 | 1

regression/sql/pgsm_query_id.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ SELECT pg_stat_monitor_reset();
2828
\c db1
2929
SELECT * FROM t1;
3030
SELECT *, add(1, 2) FROM t1;
31+
SELECT * /* x */ FROM t2;
3132
SELECT * FROM t2;
3233
-- Check that spaces and comments do not generate a different pgsm_query_id
3334
SELECT * FROM t2 --WHATEVER;

src/pg_stat_monitor.c

Lines changed: 43 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2582,11 +2582,10 @@ get_next_wbucket(pgsmSharedState *pgsm)
25822582
}
25832583

25842584
/*
2585-
* This function expects a NORMALIZED query as the input.
2586-
* It iterates over the normalized query skipping comments and
2587-
* multiple spaces. All spaces are converted to ' ' so that we
2588-
* the calculation is independent of the space type whether
2589-
* newline, tab, or any other type. Trailing and leading spaces
2585+
* This function expects a NORMALIZED query as the input. It iterates over the
2586+
* normalized query skipping comments and multiple spaces. All spaces are
2587+
* converted to ' ' so that we the calculation is independent of the space
2588+
* type whether newline, tab, or any other type. Trailing and leading spaces
25902589
* are also removed before calculating the hash.
25912590
*/
25922591
static int64
@@ -2595,82 +2594,71 @@ get_pgsm_query_id_hash(const char *norm_query, int norm_len)
25952594
char *query;
25962595
char *q_iter;
25972596
const char *norm_q_iter = norm_query;
2598-
int64 pgsm_query_id = 0;
2597+
bool space = false;
2598+
int64 pgsm_query_id;
2599+
2600+
Assert(norm_query != NULL);
25992601

26002602
if (!pgsm_enable_pgsm_query_id)
26012603
return 0;
26022604

26032605
query = palloc(norm_len + 1);
26042606
q_iter = query;
26052607

2606-
while (norm_q_iter && *norm_q_iter && norm_q_iter < norm_query + norm_len)
2608+
while (*norm_q_iter && norm_q_iter < norm_query + norm_len)
26072609
{
2608-
/*
2609-
* Skip multiline comments, + 1 is safe even if we've reach end of
2610-
* string
2611-
*/
26122610
if (*norm_q_iter == '/' && *(norm_q_iter + 1) == '*')
26132611
{
2614-
while (*norm_q_iter && *(norm_q_iter + 1) && (*norm_q_iter != '*' || *(norm_q_iter + 1) != '/'))
2615-
norm_q_iter++;
2616-
2617-
/*
2618-
* Skip the end if the current character is valid. norm_q_iter
2619-
* points to the *, we have to skip 2 characters
2620-
*/
2621-
if (*norm_q_iter)
2622-
norm_q_iter++;
2623-
if (*norm_q_iter)
2624-
norm_q_iter++;
2612+
/* Skip multiline comments */
2613+
norm_q_iter++;
2614+
norm_q_iter++;
26252615

2626-
continue;
2616+
while (*norm_q_iter)
2617+
{
2618+
if (*norm_q_iter == '*' && *(norm_q_iter + 1) == '/')
2619+
{
2620+
norm_q_iter++;
2621+
norm_q_iter++;
2622+
break;
2623+
}
2624+
else
2625+
norm_q_iter++;
2626+
}
26272627
}
2628-
2629-
/*
2630-
* Skip single line comments, + 1 is safe even if we've reach end of
2631-
* string
2632-
*/
2633-
if (*norm_q_iter == '-' && *(norm_q_iter + 1) == '-')
2628+
else if (*norm_q_iter == '-' && *(norm_q_iter + 1) == '-')
26342629
{
2630+
/* Skip single line comments */
2631+
norm_q_iter++;
2632+
norm_q_iter++;
2633+
26352634
while (*norm_q_iter && *norm_q_iter != '\n')
26362635
norm_q_iter++;
26372636
}
2638-
2639-
/* Skip white spaces */
2640-
if (scanner_isspace(*norm_q_iter))
2637+
else if (scanner_isspace(*norm_q_iter))
26412638
{
2642-
while (scanner_isspace(*++norm_q_iter));
2639+
/* Convert white spaces into single space */
2640+
norm_q_iter++;
26432641

2644-
/*
2645-
* Let's replace it with a simple space. -1 is safe as we are
2646-
* making sure we are not at the start of the string.
2647-
*/
2648-
if (q_iter != query && !scanner_isspace(*(q_iter - 1)))
2649-
*q_iter++ = ' ';
2642+
while (scanner_isspace(*norm_q_iter))
2643+
norm_q_iter++;
26502644

2651-
continue;
2645+
space = true;
2646+
}
2647+
else
2648+
{
2649+
if (space && q_iter != query)
2650+
*q_iter++ = ' ';
2651+
space = false;
2652+
*q_iter++ = *norm_q_iter++;
26522653
}
2653-
2654-
*q_iter++ = *norm_q_iter++;
26552654
}
26562655

2657-
/* Ensure we have a terminating zero at the end */
26582656
*q_iter = '\0';
26592657

2660-
/* Get rid of trailing spaces */
2661-
while (q_iter > query && *q_iter == '\0')
2662-
{
2663-
q_iter--;
2664-
2665-
/* Continue reducing the string size if space is found. */
2666-
if (scanner_isspace(*q_iter))
2667-
*q_iter = '\0';
2668-
}
2669-
2670-
/* Calculate the hash. */
2671-
pgsm_query_id = pgsm_hash_string(query, strlen(query));
2658+
pgsm_query_id = pgsm_hash_string(query, q_iter - query);
26722659

26732660
pfree(query);
2661+
26742662
return pgsm_query_id;
26752663
}
26762664

0 commit comments

Comments
 (0)