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 <ishii@postgresql.org>
Reported-by: liujinyang-highgo
Backpatch-through: v4.3
Discussion: https://github.com/pgpool/pgpool2/issues/143

diff --git a/src/rewrite/pool_timestamp.c b/src/rewrite/pool_timestamp.c
index 32de3f3096b4a99ae57097338aea40b694866933..050a0210c693110cf7b688bbfabe4edc1e08116c 100644 (file)
--- 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 d48247aabacac2bc3bcadd79ffdef300943abbc7..969e51d50a640b4633c25587886b1fa6fe3581d1 100644 (file)
--- 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);