From: David Rowley Date: Tue, 16 Jun 2026 01:42:21 +0000 (+1200) Subject: Fix various query jumble comments X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e5f94c4808fe88c170840ac3a24cdfa423b404fc;p=thirdparty%2Fpostgresql.git Fix various query jumble comments Some comments for struct WindowFunc were trying to detail which fields were irrelevant for query jumble but the list had not been kept up-to-date. Here we fix that by removing the comment to allow the "query_jumble_ignore" attribute to self-document. This involved removing similar comments from other structs. While we're on the topic, improve comments around why Consts only jumble the "consttype" and also add some rationale about why various other fields are ignored. Reported-by: jian he Author: David Rowley Author: Tom Lane Discussion: https://postgr.es/m/CACJufxEWeP2SLVMsbFNynd0pQnwbxh6U-v1nq5ccf9mSvBZntw%40mail.gmail.com --- diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index a2925ae4946..372eee20680 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -105,14 +105,12 @@ typedef enum NodeTag * - custom_query_jumble: Has custom implementation in queryjumblefuncs.c * for the field of a node. Also available as a node attribute. * - * - query_jumble_ignore: Ignore the field for the query jumbling. Note - * that typmod and collation information are usually irrelevant for the - * query jumbling. + * - query_jumble_ignore: Ignore the field for query jumbling. * * - query_jumble_squash: Squash multiple values during query jumbling. * * - query_jumble_location: Mark the field as a location to track. This is - * only allowed for integer fields that include "location" in their name. + * only used for fields of type ParseLoc, which otherwise are not jumbled. * * - read_as(VALUE): In nodeRead(), replace the field's value with VALUE. * diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 91377a6cde3..4133c404a6b 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -109,10 +109,13 @@ typedef uint64 AclMode; /* a bitmask of privilege bits */ * Planning converts a Query tree into a Plan tree headed by a PlannedStmt * node --- the Query structure is not used by the executor. * - * All the fields ignored for the query jumbling are not semantically - * significant (such as alias names), as is ignored anything that can - * be deduced from child nodes (else we'd just be double-hashing that - * piece of information). + * We ignore fields for query jumbling if they are not semantically + * significant (such as alias names). We also ignore anything that can + * be deduced from other fields or child nodes, else we'd just be + * double-hashing that piece of information. In some places query jumbling + * deliberately ignores fields that are semantically significant, such as + * Const values, because we have made a policy decision to combine queries + * that differ only in those respects. */ typedef struct Query { @@ -125,8 +128,8 @@ typedef struct Query /* * query identifier (can be set by plugins); ignored for equal, as it - * might not be set; also not stored. This is the result of the query - * jumble, hence ignored. + * might not be set; also not stored. This is the output of query + * jumbling, hence it must be ignored as an input. * * We store this as a signed value as this is the form it's displayed to * users in places such as EXPLAIN and pg_stat_statements. Primarily this @@ -142,8 +145,7 @@ typedef struct Query /* * rtable index of target relation for INSERT/UPDATE/DELETE/MERGE; 0 for - * SELECT. This is ignored in the query jumble as unrelated to the - * compilation of the query ID. + * SELECT. */ int resultRelation pg_node_attr(query_jumble_ignore); @@ -1427,8 +1429,6 @@ typedef struct RTEPermissionInfo * time. We do however remember how many columns we thought the type had * (including dropped columns!), so that we can successfully ignore any * columns added after the query was parsed. - * - * The query jumbling only needs to track the function expression. */ typedef struct RangeTblFunction { @@ -1647,9 +1647,6 @@ typedef struct GroupingSet * When refname isn't null, the partitionClause is always copied from there; * the orderClause might or might not be copied (see copiedOrder); the framing * options are never copied, per spec. - * - * The information relevant for the query jumbling is the partition clause - * type and its bounds. */ typedef struct WindowClause { @@ -1823,7 +1820,7 @@ typedef struct CommonTableExpr /* * Number of RTEs referencing this CTE (excluding internal - * self-references), irrelevant for query jumbling. + * self-references). */ int cterefcount pg_node_attr(query_jumble_ignore); /* list of output column names */ @@ -2364,7 +2361,7 @@ typedef struct SetOperationStmt Node *rarg; /* right child */ /* Eventually add fields for CORRESPONDING spec here */ - /* Fields derived during parse analysis (irrelevant for query jumbling): */ + /* Fields derived during parse analysis: */ /* OID list of output column type OIDs */ List *colTypes pg_node_attr(query_jumble_ignore); /* integer list of output column typmods */ @@ -3740,8 +3737,6 @@ typedef struct InlineCodeBlock * list contains copies of the expressions for all output arguments, in the * order of the procedure's declared arguments. (outargs is never evaluated, * but is useful to the caller as a reference for what to assign to.) - * The transformed call state is not relevant in the query jumbling, only the - * function call is. * ---------------------- */ typedef struct CallStmt diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 7977ee24783..bb05aeebee4 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -319,7 +319,10 @@ typedef struct Var * references). This ensures that the Const node is self-contained and makes * it more likely that equal() will see logically identical values as equal. * - * Only the constant type OID is relevant for the query jumbling. + * For query jumble, we don't want different const values changing the jumble + * result. We only jumble consttype as different const types could result in + * very different plans and execution times, which is useful to distinguish in + * extensions such as pg_stat_statements. */ typedef struct Const { @@ -346,10 +349,7 @@ typedef struct Const */ bool constbyval pg_node_attr(query_jumble_ignore); - /* - * token location, or -1 if unknown. All constants are tracked as - * locations in query jumbling, to be marked as parameters. - */ + /* token location, or -1 if unknown. */ ParseLoc location pg_node_attr(query_jumble_location); } Const; @@ -452,9 +452,6 @@ typedef struct Param * and can share the result. Aggregates with same 'transno' but different * 'aggno' can share the same transition state, only the final function needs * to be called separately. - * - * Information related to collations, transition types and internal states - * are irrelevant for the query jumbling. */ typedef struct Aggref { @@ -550,9 +547,6 @@ typedef struct Aggref * * In raw parse output we have only the args list; parse analysis fills in the * refs list, and the planner fills in the cols list. - * - * All the fields used as information for an internal state are irrelevant - * for the query jumbling. */ typedef struct GroupingFunc { @@ -574,13 +568,6 @@ typedef struct GroupingFunc ParseLoc location; } GroupingFunc; -/* - * WindowFunc - * - * Collation information is irrelevant for the query jumbling, as is the - * internal state information of the node like "winstar" and "winagg". - */ - /* * Null Treatment options. If specified, initially set to PARSER_IGNORE_NULLS * which is then converted to IGNORE_NULLS if the window function allows the @@ -591,6 +578,11 @@ typedef struct GroupingFunc #define PARSER_RESPECT_NULLS 2 #define IGNORE_NULLS 3 + /* + * WindowFunc + * + * Node type to represent a call to a window function. + */ typedef struct WindowFunc { Expr xpr; @@ -703,8 +695,6 @@ typedef struct MergeSupportFunc * subscripting logic. Likewise, reftypmod and refcollid will match the * container's properties in a store, but could be different in a fetch. * - * Any internal state data is ignored for the query jumbling. - * * Note: for the cases where a container is returned, if refexpr yields a R/W * expanded container, then the implementation is allowed to modify that * object in-place and return the same object. @@ -772,9 +762,6 @@ typedef enum CoercionForm /* * FuncExpr - expression node for a function call - * - * Collation information is irrelevant for the query jumbling, only the - * arguments and the function OID matter. */ typedef struct FuncExpr { @@ -839,9 +826,6 @@ typedef struct NamedArgExpr * of the node. The planner makes sure it is valid before passing the node * tree to the executor, but during parsing/planning opfuncid can be 0. * Therefore, equal() will accept a zero value as being equal to other values. - * - * Internal state information and collation data is irrelevant for the query - * jumbling. */ typedef struct OpExpr { @@ -919,9 +903,6 @@ typedef OpExpr NullIfExpr; * Similar to OpExpr, opfuncid, hashfuncid, and negfuncid are not necessarily * filled in right away, so will be ignored for equality if they are not set * yet. - * - * OID entries of the internal function types are irrelevant for the query - * jumbling, but the operator OID and the arguments are. */ typedef struct ScalarArrayOpExpr {