From 747358a97a39568e9a8ba5b6433fa464e2215318 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii Date: Thu, 22 Jan 2026 13:18:00 +0900 Subject: [PATCH] Fix walker functions to return earlier. Previously walker functions returned false once it found target objects. But it is possible to short-circuit the traverse and it should have been. Also import comments from PostgreSQL to clarify walker function design. Author: Tatsuo Ishii Reported-by: liujinyang-highgo Backpatch-through: v4.3 Discussion: https://github.com/pgpool/pgpool2/issues/143 --- src/rewrite/pool_timestamp.c | 40 +++++++++++++++++++++++++++++++++- src/utils/pool_select_walker.c | 18 +++++++-------- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/rewrite/pool_timestamp.c b/src/rewrite/pool_timestamp.c index 32de3f309..050a0210c 100644 --- a/src/rewrite/pool_timestamp.c +++ b/src/rewrite/pool_timestamp.c @@ -5,7 +5,7 @@ * pgpool: a language independent connection pool server for PostgreSQL * written by Tatsuo Ishii * - * Copyright (c) 2003-2025 PgPool Global Development Group + * Copyright (c) 2003-2026 PgPool Global Development Group * * Permission to use, copy, modify, and distribute this software and * its documentation for any purpose and without fee is hereby @@ -1369,6 +1369,44 @@ makeTypeCastFromSvfOp(SQLValueFunctionOp op) * Currently, the node type coverage extends to SelectStmt and everything * that could appear under it, but not other statement types. */ + +/* + * expression_tree_walker() is designed to support routines that traverse + * a tree in a read-only fashion (although it will also work for routines + * that modify nodes in-place but never add/delete/replace nodes). + * A walker routine should look like this: + * + * bool my_walker (Node *node, my_struct *context) + * { + * if (node == NULL) + * return false; + * // check for nodes that special work is required for, eg: + * if (IsA(node, Var)) + * { + * ... do special actions for Var nodes + * } + * else if (IsA(node, ...)) + * { + * ... do special actions for other node types + * } + * // for any node type not specially processed, do: + * return expression_tree_walker(node, my_walker, context); + * } + * + * The "context" argument points to a struct that holds whatever context + * information the walker routine needs --- it can be used to return data + * gathered by the walker, too. This argument is not touched by + * expression_tree_walker, but it is passed down to recursive sub-invocations + * of my_walker. The tree walk is started from a setup routine that + * fills in the appropriate context struct, calls my_walker with the top-level + * node of the tree, and then examines the results. + * + * The walker routine should return "false" to continue the tree walk, or + * "true" to abort the walk and immediately return "true" to the top-level + * caller. This can be used to short-circuit the traversal if the walker + * has found what it came for. "false" is returned to the top-level caller + * iff no invocation of the walker returned "true". + */ bool raw_expression_tree_walker(Node *node, tree_walker_callback walker, diff --git a/src/utils/pool_select_walker.c b/src/utils/pool_select_walker.c index d48247aab..969e51d50 100644 --- a/src/utils/pool_select_walker.c +++ b/src/utils/pool_select_walker.c @@ -3,7 +3,7 @@ * pgpool: a language independent connection pool server for PostgreSQL * written by Tatsuo Ishii * - * Copyright (c) 2003-2024 PgPool Global Development Group + * Copyright (c) 2003-2026 PgPool Global Development Group * * Permission to use, copy, modify, and distribute this software and * its documentation for any purpose and without fee is hereby @@ -416,7 +416,7 @@ function_call_walker(Node *node, void *context) if (function_volatile_property(fname, FUNC_VOLATILE)) { ctx->has_function_call = true; - return false; + return true; } return raw_expression_tree_walker(node, function_call_walker, context); } @@ -441,7 +441,7 @@ function_call_walker(Node *node, void *context) * found a writing function. */ ctx->has_function_call = true; - return false; + return true; } /* @@ -454,7 +454,7 @@ function_call_walker(Node *node, void *context) { /* Found. */ ctx->has_function_call = true; - return false; + return true; } } } @@ -483,7 +483,7 @@ system_catalog_walker(Node *node, void *context) if (is_system_catalog(rgv->relname)) { ctx->has_system_catalog = true; - return false; + return true; } } return raw_expression_tree_walker(node, system_catalog_walker, context); @@ -510,7 +510,7 @@ temp_table_walker(Node *node, void *context) if (is_temp_table(rgv->relname)) { ctx->has_temp_table = true; - return false; + return true; } } return raw_expression_tree_walker(node, temp_table_walker, context); @@ -540,7 +540,7 @@ unlogged_table_walker(Node *node, void *context) if (is_unlogged_table(relname)) { ctx->has_unlogged_table = true; - return false; + return true; } } return raw_expression_tree_walker(node, unlogged_table_walker, context); @@ -570,7 +570,7 @@ view_walker(Node *node, void *context) if (is_view(relname)) { ctx->has_view = true; - return false; + return true; } } return raw_expression_tree_walker(node, view_walker, context); @@ -600,7 +600,7 @@ row_security_enabled_walker(Node *node, void *context) if (row_security_enabled(relname)) { ctx->row_security = true; - return false; + return true; } } return raw_expression_tree_walker(node, row_security_enabled_walker, context); -- 2.39.5