From: Tatsuo Ishii Date: Tue, 14 Jul 2020 13:07:59 +0000 (+0900) Subject: Prevent data modifying CTE to be cached. X-Git-Tag: V3_6_22~29 X-Git-Url: http://git.postgresql.org/gitweb/static/gitweb.js?a=commitdiff_plain;h=24fb5d6e728efeedc8ade771fd13b8f914af32e7;p=pgpool2.git Prevent data modifying CTE to be cached. Data modifying CTE was mistakenly treated as normal read only CTE and result query was created. As a result subsequent CTE was not executed. Problem reported and patch created by Hou, Zhijie. Subtle changes to the regression test by me. Discussion: https://www.pgpool.net/pipermail/pgpool-hackers/2020-July/003705.html --- diff --git a/src/include/query_cache/pool_memqcache.h b/src/include/query_cache/pool_memqcache.h index 63e1e71ab..9c0e1a3fb 100644 --- a/src/include/query_cache/pool_memqcache.h +++ b/src/include/query_cache/pool_memqcache.h @@ -229,7 +229,8 @@ extern bool pool_is_likely_select(char *query); extern bool pool_is_table_in_black_list(const char *table_name); extern bool pool_is_table_in_white_list(const char *table_name); extern bool pool_is_allow_to_cache(Node *node, char *query); -extern int pool_extract_table_oids(Node *node, int **oidsp); +extern int pool_extract_table_oids(Node *node, int **oidsp); +extern int pool_extract_withclause_oids(Node *with, int *oidsp); extern void pool_add_dml_table_oid(int oid); extern void pool_discard_oid_maps(void); extern int pool_get_database_oid_from_dbname(char *dbname); diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c index ba7cf1c7c..3330238f9 100644 --- a/src/protocol/pool_proto_modules.c +++ b/src/protocol/pool_proto_modules.c @@ -452,8 +452,11 @@ POOL_STATUS SimpleQuery(POOL_CONNECTION *frontend, pool_unset_cache_safe(); } - /* If table is to be cached and the query is DML, save the table oid */ - if (!is_select_query && !query_context->is_parse_error) + /* + * If table is to be cached and the query is DML, save the table + * oid + */ + if (!query_context->is_parse_error) { num_oids = pool_extract_table_oids(node, &oids); diff --git a/src/query_cache/pool_memqcache.c b/src/query_cache/pool_memqcache.c index b5c2e3312..a3b4e223d 100644 --- a/src/query_cache/pool_memqcache.c +++ b/src/query_cache/pool_memqcache.c @@ -906,6 +906,27 @@ bool pool_is_allow_to_cache(Node *node, char *query) if (pool_has_unlogged_table(node)) return false; } + + /* + * If Data-modifying statements in WITH clause, it's not allowed to cache. + */ + if(IsA(node, SelectStmt) && ((SelectStmt *) node)->withClause) + { + ListCell *lc; + WithClause *withClause = ((SelectStmt *) node)->withClause; + + foreach(lc, withClause->ctes) + { + CommonTableExpr *cte = (CommonTableExpr *)lfirst(lc); + if(IsA(cte->ctequery, InsertStmt) || + IsA(cte->ctequery, DeleteStmt) || + IsA(cte->ctequery, UpdateStmt)) + { + return false; + } + } + } + return true; } @@ -942,6 +963,8 @@ bool pool_is_table_in_white_list(const char *table_name) /* * Extract table oid from INSERT/UPDATE/DELETE/TRUNCATE/ * DROP TABLE/ALTER TABLE/COPY FROM statement. + * For SELECT, if Data-modifying statements in its WITH clause, + * extract table oid from Data-modifying statements too. * Returns number of oids. * In case of error, returns 0 (InvalidOid). * oids buffer (oidsp) will be discarded by subsequent call. @@ -966,20 +989,31 @@ int pool_extract_table_oids(Node *node, int **oidsp) if (IsA(node, InsertStmt)) { - InsertStmt *stmt = (InsertStmt *)node; + InsertStmt *stmt = (InsertStmt *) node; + + num_oids = pool_extract_withclause_oids((Node *) stmt->withClause, *oidsp); table = make_table_name_from_rangevar(stmt->relation); } else if (IsA(node, UpdateStmt)) { - UpdateStmt *stmt = (UpdateStmt *)node; + UpdateStmt *stmt = (UpdateStmt *) node; + + num_oids = pool_extract_withclause_oids((Node *) stmt->withClause, *oidsp); table = make_table_name_from_rangevar(stmt->relation); } else if (IsA(node, DeleteStmt)) { - DeleteStmt *stmt = (DeleteStmt *)node; + DeleteStmt *stmt = (DeleteStmt *) node; + + num_oids = pool_extract_withclause_oids((Node *) stmt->withClause, *oidsp); table = make_table_name_from_rangevar(stmt->relation); } - + else if(IsA(node, SelectStmt)) + { + SelectStmt *stmt = (SelectStmt *) node; + num_oids = pool_extract_withclause_oids((Node *) stmt->withClause, *oidsp); + table = NULL; + } #ifdef NOT_USED /* * We do not handle CREATE TABLE here. It is possible that @@ -1100,6 +1134,73 @@ int pool_extract_table_oids(Node *node, int **oidsp) return num_oids; } +/* + * Extract table oid from INSERT/UPDATE/DELETE + * FROM statement in WITH clause. + * Returns number of oids. + * oids buffer (oidsp) will be discarded by subsequent call. + */ +int +pool_extract_withclause_oids(Node *node, int *oidsp) +{ + int num_oids = 0; + int oid; + char *table; + ListCell *lc; + WithClause *with; + + if(oidsp == NULL) + { + return 0; + } + + if(!node || !IsA(node, WithClause)) + { + return 0; + } + + with = (WithClause *) node; + foreach(lc, with->ctes) + { + CommonTableExpr *cte = (CommonTableExpr *)lfirst(lc); + if(IsA(cte->ctequery, InsertStmt)) + { + InsertStmt *stmt = (InsertStmt *) cte->ctequery; + table = make_table_name_from_rangevar(stmt->relation); + } + else if(IsA(cte->ctequery, DeleteStmt)) + { + DeleteStmt *stmt = (DeleteStmt *) cte->ctequery; + table = make_table_name_from_rangevar(stmt->relation); + } + else if(IsA(cte->ctequery, UpdateStmt)) + { + UpdateStmt *stmt = (UpdateStmt *) cte->ctequery; + table = make_table_name_from_rangevar(stmt->relation); + } + else + { + /* only check INSERT/DELETE/UPDATE in WITH clause */ + table = NULL; + } + + oid = pool_table_name_to_oid(table); + if (oid > 0) + { + if (num_oids >= POOL_MAX_DML_OIDS) + { + break; + } + + oidsp[num_oids++] = pool_table_name_to_oid(table); + ereport(DEBUG1, + (errmsg("memcache: extracting table oids: table: \"%s\" oid:%d", table, oidsp[num_oids - 1]))); + } + } + + return num_oids; +} + #define POOL_OIDBUF_SIZE 1024 static int* oidbuf; static int oidbufp; @@ -3309,14 +3410,41 @@ void pool_handle_query_cache(POOL_CONNECTION_POOL *backend, char *query, Node *n /* Non cachable SELECT */ if (node && IsA(node, SelectStmt)) { + /* Extract table oids from buffer */ + num_oids = pool_get_dml_table_oid(&oids); + if (state == 'I') { + /* + * If Data-modifying statements in SELECT's WITH clause, + * invalidate query cache. + */ + if (num_oids > 0 && pool_config->memqcache_auto_cache_invalidation) + { + POOL_SETMASK2(&BlockSig, &oldmask); + pool_shmem_lock(); + pool_invalidate_query_cache(num_oids, oids, true, 0); + pool_shmem_unlock(); + POOL_SETMASK(&oldmask); + } + /* Count up SELECT stats */ pool_stats_count_up_num_selects(1); pool_reset_memqcache_buffer(true); } else { + /* + * If we are inside a transaction, we cannot invalidate + * query cache yet. However we can clear cache buffer, if + * DML/DDL modifies the TABLE which SELECT uses. + */ + if (num_oids > 0 && pool_config->memqcache_auto_cache_invalidation) + { + pool_check_and_discard_cache_buffer(num_oids, oids); + pool_reset_memqcache_buffer(false); + } + /* Count up temporary SELECT stats */ pool_tmp_stats_count_up_num_selects(); } diff --git a/src/test/regression/tests/006.memqcache/test.sh b/src/test/regression/tests/006.memqcache/test.sh index 02566d338..0272cf4e3 100755 --- a/src/test/regression/tests/006.memqcache/test.sh +++ b/src/test/regression/tests/006.memqcache/test.sh @@ -41,6 +41,7 @@ do $PSQL test < /dev/null && success=false grep "fetched from cache" log/pgpool.log | grep normal_v > /dev/null && success=false grep "fetched from cache" log/pgpool.log | grep white_v > /dev/null || success=false + grep "fetched from cache" log/pgpool.log | grep with_modify > /dev/null && success=false if [ $success = false ];then ./shutdownall exit 1 diff --git a/src/utils/pool_select_walker.c b/src/utils/pool_select_walker.c index 89d7a98fd..29fd05a93 100644 --- a/src/utils/pool_select_walker.c +++ b/src/utils/pool_select_walker.c @@ -1200,6 +1200,12 @@ select_table_walker(Node *node, void *context) } } + /* Skip Data-Modifying Statements in SELECT. */ + else if (IsA(node, InsertStmt) || IsA(node, DeleteStmt) || IsA(node, UpdateStmt)) + { + return false; + } + return raw_expression_tree_walker(node, select_table_walker, context); }