Missing comments/docs about custom scan path
Hi,
While working on [1]/messages/by-id/CAPmGK16ah9JtyVPtdqu6d=QGkRX=RAzoYQfX7=LZ+KnqwBfftg@mail.gmail.com, I noticed $SUBJECT: commit e7cb7ee14 failed to
update comments for the CustomPath struct in pathnodes.h, and commit
f49842d1e failed to update docs about custom scan path callbacks in
custom-scan.sgml, IIUC. Attached are patches for updating these,
which I created separately for ease of review (patch
update-custom-scan-path-comments.patch for the former and patch
update-custom-scan-path-docs.patch for the latter). In the second
patch I used almost the same text as for the
ReparameterizeForeignPathByChild callback function in fdwhandler.sgml.
Best regards,
Etsuro Fujita
[1]: /messages/by-id/CAPmGK16ah9JtyVPtdqu6d=QGkRX=RAzoYQfX7=LZ+KnqwBfftg@mail.gmail.com
Attachments:
update-custom-scan-path-comments.patchapplication/octet-stream; name=update-custom-scan-path-comments.patchDownload
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index c17b53f7ad..a1dc1d07e1 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1836,15 +1836,18 @@ typedef struct ForeignPath
} ForeignPath;
/*
- * CustomPath represents a table scan done by some out-of-core extension.
+ * CustomPath represents a table scan or a table join done by some out-of-core
+ * extension.
*
* We provide a set of hooks here - which the provider must take care to set
* up correctly - to allow extensions to supply their own methods of scanning
- * a relation. For example, a provider might provide GPU acceleration, a
- * cache-based scan, or some other kind of logic we haven't dreamed up yet.
+ * a relation or joing relations. For example, a provider might provide GPU
+ * acceleration, a cache-based scan, or some other kind of logic we haven't
+ * dreamed up yet.
*
- * CustomPaths can be injected into the planning process for a relation by
- * set_rel_pathlist_hook functions.
+ * CustomPaths can be injected into the planning process for a base or join
+ * relation by set_rel_pathlist_hook or set_join_pathlist_hook functions,
+ * respectively.
*
* Core code must avoid assuming that the CustomPath is only as large as
* the structure declared here; providers are allowed to make it the first
update-custom-scan-path-docs.patchapplication/octet-stream; name=update-custom-scan-path-docs.patchDownload
diff --git a/doc/src/sgml/custom-scan.sgml b/doc/src/sgml/custom-scan.sgml
index 93d96f2f56..cd989e7d3c 100644
--- a/doc/src/sgml/custom-scan.sgml
+++ b/doc/src/sgml/custom-scan.sgml
@@ -90,7 +90,7 @@ typedef struct CustomPath
by <literal>nodeToString</literal>, so that debugging routines that attempt to
print the custom path will work as designed. <structfield>methods</structfield> must
point to a (usually statically allocated) object implementing the required
- custom path methods, of which there is currently only one.
+ custom path methods, which are further detailed below.
</para>
<para>
@@ -130,6 +130,23 @@ Plan *(*PlanCustomPath) (PlannerInfo *root,
be a <literal>CustomScan</literal> object, which the callback must allocate and
initialize. See <xref linkend="custom-scan-plan"/> for more details.
</para>
+
+ <para>
+<programlisting>
+List *(*ReparameterizeCustomPathByChild) (PlannerInfo *root,
+ List *custom_private,
+ RelOptInfo *child_rel);
+</programlisting>
+ This callback is called while converting a path parameterized by the
+ top-most parent of the given child relation <literal>child_rel</literal>
+ to be parameterized by the child relation. The callback is used to
+ reparameterize any paths or translate any expression nodes saved in the
+ given <literal>custom_private</literal> member of a
+ <structname>CustomPath</structname>. The callback may use
+ <literal>reparameterize_path_by_child</literal>,
+ <literal>adjust_appendrel_attrs</literal> or
+ <literal>adjust_appendrel_attrs_multilevel</literal> as required.
+ </para>
</sect2>
</sect1>
On Mon, Jul 31, 2023 at 7:05 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
While working on [1], I noticed $SUBJECT: commit e7cb7ee14 failed to
update comments for the CustomPath struct in pathnodes.h, and commit
f49842d1e failed to update docs about custom scan path callbacks in
custom-scan.sgml, IIUC. Attached are patches for updating these,
which I created separately for ease of review (patch
update-custom-scan-path-comments.patch for the former and patch
update-custom-scan-path-docs.patch for the latter). In the second
patch I used almost the same text as for the
ReparameterizeForeignPathByChild callback function in fdwhandler.sgml.
There seem to be no objections, so I pushed both.
Best regards,
Etsuro Fujita
On Thu, Aug 3, 2023 at 6:01 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
On Mon, Jul 31, 2023 at 7:05 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
While working on [1], I noticed $SUBJECT:
Another thing I would like to propose is minor adjustments to the docs
related to parallel query:
A custom scan provider will typically add paths for a base relation by
setting the following hook, which is called after the core code has
generated all the access paths it can for the relation (except for
Gather paths, which are made after this call so that they can use
partial paths added by the hook):
For clarity, I think "except for Gather paths" should be "except for
Gather and Gather Merge paths".
Although this hook function can be used to examine, modify, or remove
paths generated by the core system, a custom scan provider will
typically confine itself to generating CustomPath objects and adding
them to rel using add_path.
For clarity, I think "adding them to rel using add_path" should be eg,
"adding them to rel using add_path, or using add_partial_path if they
are partial paths".
Attached is a patch for that.
Best regards,
Etsuro Fujita
Attachments:
further-update-custom-scan-path-docs.patchapplication/octet-stream; name=further-update-custom-scan-path-docs.patchDownload
diff --git a/doc/src/sgml/custom-scan.sgml b/doc/src/sgml/custom-scan.sgml
index 836776b27b..a200d502cd 100644
--- a/doc/src/sgml/custom-scan.sgml
+++ b/doc/src/sgml/custom-scan.sgml
@@ -38,8 +38,8 @@
A custom scan provider will typically add paths for a base relation by
setting the following hook, which is called after the core code has
generated all the access paths it can for the relation (except for
- Gather paths, which are made after this call so that they can use
- partial paths added by the hook):
+ Gather and Gather Merge paths, which are made after this call so that
+ they can use partial paths added by the hook):
<programlisting>
typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
RelOptInfo *rel,
@@ -53,9 +53,10 @@ extern PGDLLIMPORT set_rel_pathlist_hook_type set_rel_pathlist_hook;
Although this hook function can be used to examine, modify, or remove
paths generated by the core system, a custom scan provider will typically
confine itself to generating <structname>CustomPath</structname> objects and adding
- them to <literal>rel</literal> using <function>add_path</function>. The custom scan
- provider is responsible for initializing the <structname>CustomPath</structname>
- object, which is declared like this:
+ them to <literal>rel</literal> using <function>add_path</function>, or
+ <function>add_partial_path</function> if they are partial paths. The
+ custom scan provider is responsible for initializing the
+ <structname>CustomPath</structname> object, which is declared like this:
<programlisting>
typedef struct CustomPath
{
On Tue, Aug 29, 2023 at 5:08 PM Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:
Another thing I would like to propose is minor adjustments to the docs
related to parallel query:A custom scan provider will typically add paths for a base relation by
setting the following hook, which is called after the core code has
generated all the access paths it can for the relation (except for
Gather paths, which are made after this call so that they can use
partial paths added by the hook):For clarity, I think "except for Gather paths" should be "except for
Gather and Gather Merge paths".Although this hook function can be used to examine, modify, or remove
paths generated by the core system, a custom scan provider will
typically confine itself to generating CustomPath objects and adding
them to rel using add_path.For clarity, I think "adding them to rel using add_path" should be eg,
"adding them to rel using add_path, or using add_partial_path if they
are partial paths".
+1. I can see that this change makes the doc more consistent with the
comments in set_rel_pathlist.
Thanks
Richard
Hi Richard,
On Wed, Aug 30, 2023 at 11:05 AM Richard Guo <guofenglinux@gmail.com> wrote:
On Tue, Aug 29, 2023 at 5:08 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
Another thing I would like to propose is minor adjustments to the docs
related to parallel query:A custom scan provider will typically add paths for a base relation by
setting the following hook, which is called after the core code has
generated all the access paths it can for the relation (except for
Gather paths, which are made after this call so that they can use
partial paths added by the hook):For clarity, I think "except for Gather paths" should be "except for
Gather and Gather Merge paths".Although this hook function can be used to examine, modify, or remove
paths generated by the core system, a custom scan provider will
typically confine itself to generating CustomPath objects and adding
them to rel using add_path.For clarity, I think "adding them to rel using add_path" should be eg,
"adding them to rel using add_path, or using add_partial_path if they
are partial paths".
+1. I can see that this change makes the doc more consistent with the
comments in set_rel_pathlist.
Agreed.
I have committed the patch. Thanks for taking a look!
Best regards,
Etsuro Fujita