Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vaMkb-007Uly-2n for pgsql-hackers@arkaria.postgresql.org; Mon, 29 Dec 2025 23:34:10 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vaMka-00144N-1m for pgsql-hackers@arkaria.postgresql.org; Mon, 29 Dec 2025 23:34:09 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vaMka-00144D-0g for pgsql-hackers@lists.postgresql.org; Mon, 29 Dec 2025 23:34:09 +0000 Received: from mail-yw1-x1134.google.com ([2607:f8b0:4864:20::1134]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vaMkY-003TND-18 for pgsql-hackers@lists.postgresql.org; Mon, 29 Dec 2025 23:34:08 +0000 Received: by mail-yw1-x1134.google.com with SMTP id 00721157ae682-78fba1a1b1eso111061047b3.1 for ; Mon, 29 Dec 2025 15:34:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1767051244; x=1767656044; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=koV2UUDxURFwRP8B3foiOUEuf+QcAGoFL3K7vXU6UtE=; b=hl87CKH9WHRNY1aMghda0BMFCMdF/xNcZSFZB08VMEE3ZC9irsNnq09/R8td0quiNE pATNb0pkc7q3sqO9Suiks+uPjE465zD6fMzT8XyzGL0KuC0uzl4tPLjEshCTAB7cUcu9 n1peq3wlgmhfVy9d6nzgXjuFt2ftP9kYornXbeuyubuaW0wiRJSPm6vFO2owpdjwFKj1 Dmcyv15SlJLiYrAsbbxz7rvrp7I5lF7kvVMAkR+Qio65nvjFyZ3UveyiC1p7kq9AECiC 9MovtxyTUrCxwLIBz3SrEZlAF72DkoNq7ov0+Lm8d6yy1vqWR5OXlFOqROYbNRd0BfIE 6fYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767051244; x=1767656044; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=koV2UUDxURFwRP8B3foiOUEuf+QcAGoFL3K7vXU6UtE=; b=vvk8mly8/Q0+LWLLFqN/yz1Wy6RTPwMRLDZZyIHbGgdBGfp2l3F0w/RDbIT+jsUI/b Ql+iUdNv+X42CBd2qALc8gwuusKcOeWa7zBgUoH+LrMb8NNWwNtgElemOqlhkV5tdpkO vBTvBoMxgZ5/rpsAsuHHRH/15acoEo9UfDvQSV8Q3726l/0FZjmvIrWjlvo7Kb5Q+QA6 X1yqxZS0qiQ6aQmt57WoWzwPPIP66JpTLLLOH+rCo/FbhYOstqePlAirbNlr1A/Z3zoR XfSyf/4KsIJ4WbO1TwwPgj9U7rqebZvAjG57pC4XkcsCW344/eIB7y7uIX+OGJ+ivaKn 1HTw== X-Gm-Message-State: AOJu0YwaU9tBujCpVAqABa0YRXSotPyQPWi8PWWRai070nOqaTU+n+Fv YDi0kmSmR5y2EGl9PIxOW3yPhJyaOTYiO42p6NnIBYfckT5tOJBsuhgEWYBGQYizZQMUVxPD95Q tP70uvtRitUN8IqIdgaEY51aAOXxi1t8ElmgYAdQ= X-Gm-Gg: AY/fxX71Ho7BYnnE51JTnirT/UNPgYy3rovAdS67/OzFpiyubUYLpq00zjlNhefc0H1 M/AKf20YBioPDeZLEaEGYz0/zY4KfHvuY9My/XI7ltZ2c0I3NQZKKrkQ6gE0U21L/16NMbkj7Qp gP0g9guY88QUHwTZLJaVTZvlYyTJTazSNpz5+pTB6hXw/bxyDzk4Ibf7H6sLMtdpogAAoL4WbOj 8k0CujyuZC2qwozz3Ou0vru51Jv7oiT2bnfLU4GvjioH0ELLuJhbIYr5gs1g53AI5Z5NjVUdb92 xC0= X-Google-Smtp-Source: AGHT+IFNQJGSDB9FNy8cHHF8+Vy0hmMNhzJE/R1IcnfF09JX0fMAeHz/rf5KsrMgAR4eFJCTRC+J3xM2wzP4mEXTZ9s= X-Received: by 2002:a05:690c:5042:b0:78f:86cd:5626 with SMTP id 00721157ae682-78fa5b3a090mr249214577b3.26.1767051244392; Mon, 29 Dec 2025 15:34:04 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Haibo Yan Date: Mon, 29 Dec 2025 15:33:53 -0800 X-Gm-Features: AQt7F2qlnM46JYH7FCvEoSfsTxngAABf0Ss_3ZAS-mrnHWX_R0PIrKbZj8Ifulc Message-ID: Subject: Re: pg_plan_advice To: Robert Haas Cc: PostgreSQL Hackers Content-Type: multipart/alternative; boundary="000000000000b8d46e06471fad76" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000b8d46e06471fad76 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Robert, Thank you very much for your work on the pg_plan_advice patch series. It is an impressive and substantial contribution, and it seems like a meaningful step forward toward addressing long-standing query plan stability issues in PostgreSQL. While reviewing the v7 patches, I noticed a few points that I wanted to raise for discussion: 1. GEQO interaction (patch 4): Since GEQO relies on randomized search, is there a risk that the optimizer may fail to explore the specific join order or path that is being enforced by the advice mask? In that case, could this lead to failures such as inability to construct the required join relation or excessive planning time if the desired path is not sampled? 2. Parallel query serialization (patches 1=E2=80=933): Several new fields (subrtinfos, elidedNodes, child_append_relid_sets) are added to PlannedStmt, but I did not see corresponding changes in outfuncs.c / readfuncs.c. Without serialization support, parallel workers executing subplans or Append nodes may not receive this metadata. Is this handled elsewhere, or is it something still pending? 3. Alias handling when generating advice (patch 5): In pgpa_output_relation_name, the advice string is generated using get_rel_name(relid), which resolves to the underlying table name rather than the RTE alias. In self-join cases this could be ambiguous (e.g., my_table vs my_table). Would it be more appropriate to use the RTE alias when available? 4. Minor typo (patch 4): In src/include/nodes/relation.h, parititonwise appears to be a typo and should likely be partitionwise. I hope these comments are helpful, and I apologize in advance if any of this is already addressed elsewhere in the series. Best regards, Haibo On Mon, Dec 15, 2025 at 12:06=E2=80=AFPM Robert Haas wrote: > Here's v7. > > In 0001, I removed "const" from a node's struct declaration, because > Tom gave me some feedback to avoid that on another recent patch, and I > noticed I had done it here also. 0002, 0003, and 0004 are unchanged. > > In 0005: > > - Refactored the code to avoid issuing SEMIJOIN_NON_UNIQUE() advice in > cases where uniqueness wasn't actually considered. > - Adjusted the code not to issue NO_GATHER() advice for non-relation > RTEs. (This is the issue reported by Ajay Pal in a recent message to > this thread, which was also mentioned in an XXX in the code.) > - Reject zero-length delimited identifiers, per Jacob's email. > - Properly initialize element->indexes in pgpa_trove_add_to_hash, per > Jacob'e email. > - Add gather((d d/d.d)) test case, per Jacob, and fix the related bug > in pgpa_identifier_matches_target, per Jacob's email. > - Add EXPLAIN SELECT 1 after various test cases in syntax.sql, to > improve test coverage, per analysis of why the existing test case > didn't catch a bug previously reported by Jacob. > - Added a dummy initialization to pgpa_collector.c to placate nervous > compilers, per discussion with Jacob. > > I think this mostly catches me up on responding to issues reported > here, although there is one thing reported to me off-list that I > haven't dealt with yet. If there's anything reported on thread that > isn't addressed here, let me know. > > Thanks, > > -- > Robert Haas > EDB: http://www.enterprisedb.com > --000000000000b8d46e06471fad76 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Robert,

Thank you very much for your work on the= pg_plan_advice patch series. It is an impressive and substantial contribut= ion, and it seems like a meaningful step forward toward addressing long-sta= nding query plan stability issues in PostgreSQL.

While reviewing the= v7 patches, I noticed a few points that I wanted to raise for discussion:<= br> 1. GEQO interaction (patch 4):
Since GEQO relies on randomized searc= h, is there a risk that the optimizer may fail to explore the specific join= order or path that is being enforced by the advice mask? In that case, cou= ld this lead to failures such as inability to construct the required join r= elation or excessive planning time if the desired path is not sampled?
= 2. Parallel query serialization (patches 1=E2=80=933):
Several new field= s (subrtinfos, elidedNodes, child_append_relid_sets) are added to PlannedSt= mt, but I did not see corresponding changes in outfuncs.c / readfuncs.c. Wi= thout serialization support, parallel workers executing subplans or Append = nodes may not receive this metadata. Is this handled elsewhere, or is it so= mething still pending?
3. Alias handling when generating advice (patch = 5):
In pgpa_output_relation_name, the advice string is generated using g= et_rel_name(relid), which resolves to the underlying table name rather than= the RTE alias. In self-join cases this could be ambiguous (e.g., my_table = vs my_table). Would it be more appropriate to use the RTE alias when availa= ble?
4. Minor typo (patch 4):
In src/include/nodes/relation.h, parit= itonwise appears to be a typo and should likely be partitionwise.

I = hope these comments are helpful, and I apologize in advance if any of this = is already addressed elsewhere in the series.

Best regards,
Haibo=

On Mon, Dec 15, 2025 at 12:06=E2=80=AFPM Robert Haas = <robertmhaas@gmail.com> = wrote:
Here'= s v7.

In 0001, I removed "const" from a node's struct declaration, = because
Tom gave me some feedback to avoid that on another recent patch, and I
noticed I had done it here also. 0002, 0003, and 0004 are unchanged.

In 0005:

- Refactored the code to avoid issuing SEMIJOIN_NON_UNIQUE() advice in
cases where uniqueness wasn't actually considered.
- Adjusted the code not to issue NO_GATHER() advice for non-relation
RTEs. (This is the issue reported by Ajay Pal in a recent message to
this thread, which was also mentioned in an XXX in the code.)
- Reject zero-length delimited identifiers, per Jacob's email.
- Properly initialize element->indexes in pgpa_trove_add_to_hash, per Jacob'e email.
- Add gather((d d/d.d)) test case, per Jacob, and fix the related bug
in pgpa_identifier_matches_target, per Jacob's email.
- Add EXPLAIN SELECT 1 after various test cases in syntax.sql, to
improve test coverage, per analysis of why the existing test case
didn't catch a bug previously reported by Jacob.
- Added a dummy initialization to pgpa_collector.c to placate nervous
compilers, per discussion with Jacob.

I think this mostly catches me up on responding to issues reported
here, although there is one thing reported to me off-list that I
haven't dealt with yet. If there's anything reported on thread that=
isn't addressed here, let me know.

Thanks,

--
Robert Haas
EDB: http://www.enterprisedb.com
--000000000000b8d46e06471fad76--