public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v6] fix psql display of FKs
6+ messages / 3 participants
[nested] [flat]
* [PATCH v6] fix psql display of FKs
@ 2019-03-22 22:13 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Alvaro Herrera @ 2019-03-22 22:13 UTC (permalink / raw)
---
src/bin/psql/describe.c | 140 ++++++++++++++++------
src/test/regress/expected/foreign_key.out | 10 +-
2 files changed, 110 insertions(+), 40 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index fd8ebee8cd3..734a718af21 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1479,6 +1479,7 @@ describeOneTableDetails(const char *schemaname,
bool rowsecurity;
bool forcerowsecurity;
bool hasoids;
+ bool ispartition;
Oid tablespace;
char *reloptions;
char *reloftype;
@@ -1502,7 +1503,25 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
- "false AS relhasoids, %s, c.reltablespace, "
+ "false AS relhasoids, c.relispartition, %s, c.reltablespace, "
+ "CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
+ "c.relpersistence, c.relreplident, am.amname\n"
+ "FROM pg_catalog.pg_class c\n "
+ "LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n"
+ "LEFT JOIN pg_catalog.pg_am am ON (c.relam = am.oid)\n"
+ "WHERE c.oid = '%s';",
+ (verbose ?
+ "pg_catalog.array_to_string(c.reloptions || "
+ "array(select 'toast.' || x from pg_catalog.unnest(tc.reloptions) x), ', ')\n"
+ : "''"),
+ oid);
+ }
+ else if (pset.sversion >= 110000)
+ {
+ printfPQExpBuffer(&buf,
+ "SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
+ "c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
+ "c.relhasoids, c.relispartition, %s, c.reltablespace, "
"CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
"c.relpersistence, c.relreplident, am.amname\n"
"FROM pg_catalog.pg_class c\n "
@@ -1520,7 +1539,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
- "c.relhasoids, %s, c.reltablespace, "
+ "c.relhasoids, false, %s, c.reltablespace, "
"CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
"c.relpersistence, c.relreplident\n"
"FROM pg_catalog.pg_class c\n "
@@ -1537,7 +1556,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
- "%s, c.reltablespace, "
+ "false, %s, c.reltablespace, "
"CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
"c.relpersistence, c.relreplident\n"
"FROM pg_catalog.pg_class c\n "
@@ -1554,7 +1573,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
- "%s, c.reltablespace, "
+ "false, %s, c.reltablespace, "
"CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, "
"c.relpersistence\n"
"FROM pg_catalog.pg_class c\n "
@@ -1571,7 +1590,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
- "%s, c.reltablespace, "
+ "false, %s, c.reltablespace, "
"CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END\n"
"FROM pg_catalog.pg_class c\n "
"LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n"
@@ -1587,7 +1606,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
- "%s, c.reltablespace\n"
+ "false, %s, c.reltablespace\n"
"FROM pg_catalog.pg_class c\n "
"LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n"
"WHERE c.oid = '%s';",
@@ -1602,7 +1621,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT relchecks, relkind, relhasindex, relhasrules, "
"reltriggers <> 0, false, false, relhasoids, "
- "%s, reltablespace\n"
+ "false, %s, reltablespace\n"
"FROM pg_catalog.pg_class WHERE oid = '%s';",
(verbose ?
"pg_catalog.array_to_string(reloptions, E', ')" : "''"),
@@ -1613,7 +1632,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT relchecks, relkind, relhasindex, relhasrules, "
"reltriggers <> 0, false, false, relhasoids, "
- "'', reltablespace\n"
+ "false, '', reltablespace\n"
"FROM pg_catalog.pg_class WHERE oid = '%s';",
oid);
}
@@ -1622,7 +1641,7 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&buf,
"SELECT relchecks, relkind, relhasindex, relhasrules, "
"reltriggers <> 0, false, false, relhasoids, "
- "'', ''\n"
+ "false, '', ''\n"
"FROM pg_catalog.pg_class WHERE oid = '%s';",
oid);
}
@@ -1647,20 +1666,21 @@ describeOneTableDetails(const char *schemaname,
tableinfo.rowsecurity = strcmp(PQgetvalue(res, 0, 5), "t") == 0;
tableinfo.forcerowsecurity = strcmp(PQgetvalue(res, 0, 6), "t") == 0;
tableinfo.hasoids = strcmp(PQgetvalue(res, 0, 7), "t") == 0;
+ tableinfo.ispartition = strcmp(PQgetvalue(res, 0, 8), "t") == 0;
tableinfo.reloptions = (pset.sversion >= 80200) ?
- pg_strdup(PQgetvalue(res, 0, 8)) : NULL;
+ pg_strdup(PQgetvalue(res, 0, 9)) : NULL;
tableinfo.tablespace = (pset.sversion >= 80000) ?
- atooid(PQgetvalue(res, 0, 9)) : 0;
+ atooid(PQgetvalue(res, 0, 10)) : 0;
tableinfo.reloftype = (pset.sversion >= 90000 &&
- strcmp(PQgetvalue(res, 0, 10), "") != 0) ?
- pg_strdup(PQgetvalue(res, 0, 10)) : NULL;
+ strcmp(PQgetvalue(res, 0, 11), "") != 0) ?
+ pg_strdup(PQgetvalue(res, 0, 11)) : NULL;
tableinfo.relpersistence = (pset.sversion >= 90100) ?
- *(PQgetvalue(res, 0, 11)) : 0;
+ *(PQgetvalue(res, 0, 12)) : 0;
tableinfo.relreplident = (pset.sversion >= 90400) ?
- *(PQgetvalue(res, 0, 12)) : 'd';
+ *(PQgetvalue(res, 0, 13)) : 'd';
if (pset.sversion >= 120000)
- tableinfo.relam = PQgetisnull(res, 0, 13) ?
- (char *) NULL : pg_strdup(PQgetvalue(res, 0, 13));
+ tableinfo.relam = PQgetisnull(res, 0, 14) ?
+ (char *) NULL : pg_strdup(PQgetvalue(res, 0, 14));
else
tableinfo.relam = NULL;
PQclear(res);
@@ -2394,12 +2414,33 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
- printfPQExpBuffer(&buf,
- "SELECT conname,\n"
- " pg_catalog.pg_get_constraintdef(r.oid, true) as condef\n"
- "FROM pg_catalog.pg_constraint r\n"
- "WHERE r.conrelid = '%s' AND r.contype = 'f' ORDER BY 1;",
- oid);
+ if (pset.sversion >= 120000 &&
+ (tableinfo.ispartition || tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
+ {
+ /*
+ * Note we put the constraints defined in this table first,
+ * followed by the constraints defined in ancestor partitioned
+ * tables.
+ */
+ printfPQExpBuffer(&buf,
+ "SELECT conrelid = '%s'::pg_catalog.regclass, conname,\n"
+ " pg_catalog.pg_get_constraintdef(oid, true),\n"
+ " conrelid::pg_catalog.regclass\n"
+ " FROM pg_constraint, pg_partition_ancestors('%s')\n"
+ " WHERE conrelid = relid AND contype = 'f' AND conparentid = 0\n"
+ "ORDER BY conname;",
+ oid, oid);
+ }
+ else
+ {
+ printfPQExpBuffer(&buf,
+ "SELECT true, conname,\n"
+ " pg_catalog.pg_get_constraintdef(r.oid, true) as condef\n"
+ "FROM pg_catalog.pg_constraint r\n"
+ "WHERE r.conrelid = '%s' AND r.contype = 'f' ORDER BY 1;",
+ oid);
+ }
+
result = PSQLexec(buf.data);
if (!result)
goto error_return;
@@ -2411,10 +2452,20 @@ describeOneTableDetails(const char *schemaname,
printTableAddFooter(&cont, _("Foreign-key constraints:"));
for (i = 0; i < tuples; i++)
{
- /* untranslated constraint name and def */
- printfPQExpBuffer(&buf, " \"%s\" %s",
- PQgetvalue(result, i, 0),
- PQgetvalue(result, i, 1));
+ /*
+ * Print untranslated constraint name and definition.
+ * Use a "TABLE tab" prefix when the constraint is
+ * defined in a parent partitioned table.
+ */
+ if (strcmp(PQgetvalue(result, i, 0), "f") == 0)
+ printfPQExpBuffer(&buf, " TABLE \"%s\" CONSTRAINT \"%s\" %s",
+ PQgetvalue(result, i, 3),
+ PQgetvalue(result, i, 1),
+ PQgetvalue(result, i, 2));
+ else
+ printfPQExpBuffer(&buf, " \"%s\" %s",
+ PQgetvalue(result, i, 1),
+ PQgetvalue(result, i, 2));
printTableAddFooter(&cont, buf.data);
}
@@ -2422,15 +2473,34 @@ describeOneTableDetails(const char *schemaname,
PQclear(result);
}
- /* print incoming foreign-key references (none if no triggers) */
- if (tableinfo.hastriggers)
+ /* print incoming foreign-key references */
+ if (tableinfo.hastriggers ||
+ tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
- printfPQExpBuffer(&buf,
- "SELECT conname, conrelid::pg_catalog.regclass,\n"
- " pg_catalog.pg_get_constraintdef(c.oid, true) as condef\n"
- "FROM pg_catalog.pg_constraint c\n"
- "WHERE c.confrelid = '%s' AND c.contype = 'f' ORDER BY 1;",
- oid);
+ if (pset.sversion >= 120000 &&
+ (tableinfo.ispartition ||
+ tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
+ {
+ printfPQExpBuffer(&buf,
+ "SELECT conname,\n"
+ " conrelid::pg_catalog.regclass,\n"
+ " pg_catalog.pg_get_constraintdef(oid, true)\n"
+ " FROM pg_constraint, pg_partition_ancestors('%s')\n"
+ " WHERE confrelid = relid AND contype = 'f' AND conparentid = 0\n"
+ "ORDER BY conname;",
+ oid);
+ }
+ else
+ {
+ printfPQExpBuffer(&buf,
+ "SELECT conname, conrelid::pg_catalog.regclass,\n"
+ " pg_catalog.pg_get_constraintdef(c.oid, true) as condef\n"
+ "FROM pg_catalog.pg_constraint c\n"
+ "WHERE c.confrelid = '%s' AND c.contype = 'f' AND conparentid = 0\n"
+ "ORDER BY 1;",
+ oid);
+ }
+
result = PSQLexec(buf.data);
if (!result)
goto error_return;
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 401514a3e00..eac5c0ab145 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1916,7 +1916,7 @@ alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey;
a | integer | | |
Partition of: fkpart0.fk_part FOR VALUES IN (1)
Foreign-key constraints:
- "fk_part_1_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
+ TABLE "fkpart0.fk_part" CONSTRAINT "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey;
ERROR: cannot drop inherited constraint "fk_part_1_a_fkey" of relation "fk_part_1"
@@ -1928,7 +1928,7 @@ ERROR: cannot drop inherited constraint "fk_part_1_a_fkey" of relation "fk_part
Partition of: fkpart0.fk_part FOR VALUES IN (2, 3)
Partition key: LIST (a)
Foreign-key constraints:
- "fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
+ TABLE "fkpart0.fk_part" CONSTRAINT "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
Number of partitions: 1 (Use \d+ to list them.)
\d fkpart0.fk_part_23_2 \\ -- should have only one FK
@@ -1938,7 +1938,7 @@ Number of partitions: 1 (Use \d+ to list them.)
a | integer | | |
Partition of: fkpart0.fk_part_23 FOR VALUES IN (2)
Foreign-key constraints:
- "fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
+ TABLE "fkpart0.fk_part" CONSTRAINT "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey;
ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23"
@@ -1952,7 +1952,7 @@ create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4);
a | integer | | |
Partition of: fkpart0.fk_part FOR VALUES IN (4)
Foreign-key constraints:
- "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
+ TABLE "fkpart0.fk_part" CONSTRAINT "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey;
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_4"
@@ -1968,7 +1968,7 @@ create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56
Partition of: fkpart0.fk_part FOR VALUES IN (5, 6)
Partition key: LIST (a)
Foreign-key constraints:
- "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
+ TABLE "fkpart0.fk_part" CONSTRAINT "fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
Number of partitions: 1 (Use \d+ to list them.)
alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey;
--
2.17.1
--YZ5djTAD1cGYuMQK--
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Report planning memory in EXPLAIN ANALYZE
@ 2023-12-11 13:35 Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Ashutosh Bapat @ 2023-12-11 13:35 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Alvaro Herrera <[email protected]>; Andy Fan <[email protected]>; David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Dec 11, 2023 at 2:06 PM jian he <[email protected]> wrote:
>
> On Mon, Dec 4, 2023 at 3:24 PM Ashutosh Bapat
> <[email protected]> wrote:
> >
> > On Fri, Dec 1, 2023 at 8:27 AM Andrei Lepikhov
> > <[email protected]> wrote:
> > >
> > > On 30/11/2023 18:40, Alvaro Herrera wrote:
> > > > Well, SUMMARY is enabled by default with ANALYZE, and I'd rather not
> > > > have planner memory consumption displayed by default with all EXPLAIN
> > > > ANALYZEs. So yeah, I still think this deserves its own option.
> > > >
> > > > But let's hear others' opinions on this point. I'm only one vote here.
> > >
> > > I agree; it should be disabled by default. The fact that memory
> > > consumption outputs with byte precision (very uncomfortable for
> > > perception) is a sign that the primary audience is developers and for
> > > debugging purposes.
> >
> > That's 2 vs 1. Here's patch with MEMORY option added. Replying to
> > Alvaro's earlier relevant comments.
> >
>
> "Include information on planner's memory consumption. Specially,
> include the total memory allocated by the planner and net memory that
> remains used at the end of the planning. It defaults to
> <literal>FALSE</literal>.
> "
> doc/src/sgml/ref/explain.sgml
> I can view MemoryContextSizeDifference, figure out the meaning.
>
> But I am not sure "net memory that remains used at the end of the
> planning" is the correct description.
> (I am not native english speaker).
The word "net" is used as an adjective, see [1]
[1] https://www.merriam-webster.com/dictionary/net (as an adjective)
Does that help? Do you have any other wording proposal?
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Report planning memory in EXPLAIN ANALYZE
@ 2023-12-12 20:11 Alvaro Herrera <[email protected]>
parent: Ashutosh Bapat <[email protected]>
0 siblings, 2 replies; 6+ messages in thread
From: Alvaro Herrera @ 2023-12-12 20:11 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; +Cc: jian he <[email protected]>; Andrei Lepikhov <[email protected]>; Andy Fan <[email protected]>; David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
I would replace the text in explain.sgml with this:
+ Include information on memory consumption by the query planning phase.
+ This includes the precise amount of storage used by planner in-memory
+ structures, as well as overall total consumption of planner memory,
+ including allocation overhead.
+ This parameter defaults to </literal>FALSE</literal>.
and remove the new example you're adding; it's not really necessary.
In struct ExplainState, I'd put the new member just below summary.
If we're creating a new memory context for planner, we don't need the
"mem_counts_start" thing, and we don't need the
MemoryContextSizeDifference thing. Just add the
MemoryContextMemConsumed() function (which ISTM should be just below
MemoryContextMemAllocated() instead of in the middle of the
MemoryContextStats implementation), and
just report the values we get there. I think the SizeDifference stuff
increases surface damage for no good reason.
ExplainOnePlan() is given a MemoryUsage object (or, if we do as above
and no longer have a MemoryUsage struct at all in the first place, a
MemoryContextCounters object) even when the MEMORY option is false.
This means we waste computing memory usage when not needed. Let's avoid
that useless overhead.
I'd also do away with the comment you added in explain_ExecutorEnd() and
do just this, below setting of es->summary:
+ /* No support for MEMORY option */
+ /* es->memory = false; */
We don't need to elaborate more at present.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Doing what he did amounts to sticking his fingers under the hood of the
implementation; if he gets his fingers burnt, it's his problem." (Tom Lane)
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Report planning memory in EXPLAIN ANALYZE
@ 2023-12-12 20:20 Alvaro Herrera <[email protected]>
parent: Alvaro Herrera <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Alvaro Herrera @ 2023-12-12 20:20 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; +Cc: jian he <[email protected]>; Andrei Lepikhov <[email protected]>; Andy Fan <[email protected]>; David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2023-Dec-12, Alvaro Herrera wrote:
> I would replace the text in explain.sgml with this:
>
> + Include information on memory consumption by the query planning phase.
> + This includes the precise amount of storage used by planner in-memory
> + structures, as well as overall total consumption of planner memory,
> + including allocation overhead.
> + This parameter defaults to </literal>FALSE</literal>.
(This can still use a lot more wordsmithing of course, such as avoiding
repeated use of "include/ing", ugh)
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Report planning memory in EXPLAIN ANALYZE
@ 2023-12-13 10:51 Ashutosh Bapat <[email protected]>
parent: Alvaro Herrera <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Ashutosh Bapat @ 2023-12-13 10:51 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: jian he <[email protected]>; Andrei Lepikhov <[email protected]>; Andy Fan <[email protected]>; David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Dec 13, 2023 at 1:41 AM Alvaro Herrera <[email protected]> wrote:
>
> I would replace the text in explain.sgml with this:
>
> + Include information on memory consumption by the query planning phase.
> + This includes the precise amount of storage used by planner in-memory
> + structures, as well as overall total consumption of planner memory,
> + including allocation overhead.
> + This parameter defaults to </literal>FALSE</literal>.
To me consumption in the "... total consumption ..." part is same as
allocation and that includes allocation overhead as well as any memory
that was allocated but remained unused (see discussion [1] if you
haven't already) ultimately. Mentioning "total consumption" and
"allocation overhead" seems more confusing.
How about
+ Include information on memory consumption by the query planning phase.
+ Report the precise amount of storage used by planner in-memory
+ structures, and total memory considering allocation overhead.
+ The parameter defaults to <literal>FALSE</literal>.
Takes care of your complaint about multiple include/ing as well.
>
> and remove the new example you're adding; it's not really necessary.
Done.
>
> In struct ExplainState, I'd put the new member just below summary.
Done
>
> If we're creating a new memory context for planner, we don't need the
> "mem_counts_start" thing, and we don't need the
> MemoryContextSizeDifference thing. Just add the
> MemoryContextMemConsumed() function (which ISTM should be just below
> MemoryContextMemAllocated() instead of in the middle of the
> MemoryContextStats implementation), and
> just report the values we get there. I think the SizeDifference stuff
> increases surface damage for no good reason.
240 bytes are used right after creating a memory context i.e.
mem_counts_start.totalspace = 8192 and mem_counts_start.freespace =
7952. To account for that I used two counters and calculated the
difference. If no planning is involved in EXECUTE <prepared statement>
it will show 240 bytes used instead of 0. Barring that for all
practical purposes 240 bytes negligible. E.g. 240 bytes is 4% of the
amount of memory used for planning a simple query " select 'x' ". But
your suggestion simplifies the code a lot. An error of 240 bytes seems
worth the code simplification. So did that way.
>
> ExplainOnePlan() is given a MemoryUsage object (or, if we do as above
> and no longer have a MemoryUsage struct at all in the first place, a
> MemoryContextCounters object) even when the MEMORY option is false.
> This means we waste computing memory usage when not needed. Let's avoid
> that useless overhead.
Done.
Also avoided creating a memory context and switching to it when
es->memory = false.
>
> I'd also do away with the comment you added in explain_ExecutorEnd() and
> do just this, below setting of es->summary:
>
> + /* No support for MEMORY option */
> + /* es->memory = false; */
Done.
I ended up rewriting most of the code, so squashed everything into a
single patch as attached.
--
Best Wishes,
Ashutosh Bapat
Attachments:
[text/x-patch] 0001-EXPLAIN-reports-memory-consumed-for-plannin-20231213.patch (16.1K, ../../CAExHW5u+Znp3GFgkRoQW_u1HE2=ieoHK2dtE79qhV365BB0sow@mail.gmail.com/2-0001-EXPLAIN-reports-memory-consumed-for-plannin-20231213.patch)
download | inline diff:
From 8282b3347a8535cfe09b5f2f4a0c5a6bdcba11fa Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Wed, 12 Jul 2023 14:34:14 +0530
Subject: [PATCH] EXPLAIN reports memory consumed for planning a query
The memory consumed is reported as "Planner Memory" property in EXPLAIN
output when option MEMORY is specified.
A separate memory context is allocated for measuring memory consumption
to eliminate any effect of previous activity on the calculation. The
memory context statistics is noted before and after calling
pg_plan_query() from ExplainOneQuery(). The difference in the two
statistics is used to calculate the memory consumption.
We report two values "used" and "allocated". The difference between
memory statistics counters totalspace and freespace gives the memory
that remains palloc'ed at the end of planning. It is reported as memory
"used". This does not account for chunks of memory that were palloc'ed
but subsequently pfree'ed. But such memory remains allocated in the
memory context is given by the totalspace counter. The value of this
counter is reported as memory "allocated".
Ashutosh Bapat, reviewed by David Rowley and Alvaro Herrera
---
contrib/auto_explain/auto_explain.c | 2 +
doc/src/sgml/ref/explain.sgml | 13 +++++
src/backend/commands/explain.c | 67 +++++++++++++++++++++++++-
src/backend/commands/prepare.c | 26 +++++++++-
src/backend/utils/mmgr/mcxt.c | 13 +++++
src/include/commands/explain.h | 4 +-
src/include/utils/memutils.h | 2 +
src/test/regress/expected/explain.out | 68 +++++++++++++++++++++++++++
src/test/regress/sql/explain.sql | 6 +++
9 files changed, 197 insertions(+), 4 deletions(-)
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index c3ac27ae99..26e80e4e16 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -396,6 +396,8 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
es->wal = (es->analyze && auto_explain_log_wal);
es->timing = (es->analyze && auto_explain_log_timing);
es->summary = es->analyze;
+ /* No support for MEMORY option */
+ /* es->memory = false; */
es->format = auto_explain_log_format;
es->settings = auto_explain_log_settings;
diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index 5ba6486da1..49c61cce69 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -44,6 +44,7 @@ EXPLAIN [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] <rep
WAL [ <replaceable class="parameter">boolean</replaceable> ]
TIMING [ <replaceable class="parameter">boolean</replaceable> ]
SUMMARY [ <replaceable class="parameter">boolean</replaceable> ]
+ MEMORY [ <replaceable class="parameter">boolean</replaceable> ]
FORMAT { TEXT | XML | JSON | YAML }
</synopsis>
</refsynopsisdiv>
@@ -250,6 +251,18 @@ ROLLBACK;
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>MEMORY</literal></term>
+ <listitem>
+ <para>
+ Include information on memory consumption by the query planning phase.
+ Report the precise amount of storage used by planner in-memory
+ structures, and total memory considering allocation overhead.
+ The parameter defaults to <literal>FALSE</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>FORMAT</literal></term>
<listitem>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index f1d71bc54e..0a18a7abd8 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -122,6 +122,8 @@ static const char *explain_get_index_name(Oid indexId);
static void show_buffer_usage(ExplainState *es, const BufferUsage *usage,
bool planning);
static void show_wal_usage(ExplainState *es, const WalUsage *usage);
+static void show_planner_memory(ExplainState *es,
+ const MemoryContextCounters *mem_counts);
static void ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir,
ExplainState *es);
static void ExplainScanTarget(Scan *plan, ExplainState *es);
@@ -202,6 +204,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt,
summary_set = true;
es->summary = defGetBoolean(opt);
}
+ else if (strcmp(opt->defname, "memory") == 0)
+ es->memory = defGetBoolean(opt);
else if (strcmp(opt->defname, "format") == 0)
{
char *p = defGetString(opt);
@@ -397,6 +401,24 @@ ExplainOneQuery(Query *query, int cursorOptions,
planduration;
BufferUsage bufusage_start,
bufusage;
+ MemoryContextCounters mem_counts;
+ MemoryContext planner_ctx = NULL;
+ MemoryContext saved_ctx = NULL;
+
+ if (es->memory)
+ {
+ /*
+ * Create a new memory context to measure planner's memory
+ * consumption accurately. We should use the same type of memory
+ * context as the planner would use. That's usually AllocSet but
+ * ensure that.
+ */
+ Assert(IsA(CurrentMemoryContext, AllocSetContext));
+ planner_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "explain analyze planner context",
+ ALLOCSET_DEFAULT_SIZES);
+ saved_ctx = MemoryContextSwitchTo(planner_ctx);
+ }
if (es->buffers)
bufusage_start = pgBufferUsage;
@@ -408,6 +430,12 @@ ExplainOneQuery(Query *query, int cursorOptions,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ if (es->memory)
+ {
+ MemoryContextSwitchTo(saved_ctx);
+ MemoryContextMemConsumed(planner_ctx, &mem_counts);
+ }
+
/* calc differences of buffer counters. */
if (es->buffers)
{
@@ -417,7 +445,8 @@ ExplainOneQuery(Query *query, int cursorOptions,
/* run it (if needed) and produce output */
ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL));
+ &planduration, (es->buffers ? &bufusage : NULL),
+ (es->memory ? &mem_counts : NULL));
}
}
@@ -527,7 +556,8 @@ void
ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
- const BufferUsage *bufusage)
+ const BufferUsage *bufusage,
+ const MemoryContextCounters *mem_counts)
{
DestReceiver *dest;
QueryDesc *queryDesc;
@@ -630,6 +660,13 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
}
+ if (mem_counts)
+ {
+ ExplainOpenGroup("Planner Memory", "Planner Memory", true, es);
+ show_planner_memory(es, mem_counts);
+ ExplainCloseGroup("Planner Memory", "Planner Memory", true, es);
+ }
+
/* Print info about runtime of triggers */
if (es->analyze)
ExplainPrintTriggers(es, queryDesc);
@@ -3766,6 +3803,32 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
}
}
+/*
+ * Show planner's memory usage details.
+ */
+static void
+show_planner_memory(ExplainState *es,
+ const MemoryContextCounters *mem_counts)
+{
+ if (es->format == EXPLAIN_FORMAT_TEXT)
+ {
+ appendStringInfo(es->str,
+ "Planner Memory: used=%zu bytes allocated=%zu bytes",
+ mem_counts->totalspace - mem_counts->freespace,
+ mem_counts->totalspace);
+ appendStringInfoChar(es->str, '\n');
+ }
+ else
+ {
+ ExplainPropertyInteger("Used", "bytes",
+ mem_counts->totalspace - mem_counts->freespace,
+ es);
+ ExplainPropertyInteger("Allocated", "bytes",
+ mem_counts->totalspace, es);
+ }
+}
+
+
/*
* Add some additional details about an IndexScan or IndexOnlyScan
*/
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 18f70319fc..7e947e023b 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -583,6 +583,23 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
instr_time planduration;
BufferUsage bufusage_start,
bufusage;
+ MemoryContextCounters mem_counts;
+ MemoryContext planner_ctx = NULL;
+ MemoryContext saved_ctx = NULL;
+
+ if (es->memory)
+ {
+ /*
+ * Create a new memory context to measure planner's memory consumption
+ * accurately. We should use the same type of memory context as the
+ * planner would use. That's usually AllocSet but ensure that.
+ */
+ Assert(IsA(CurrentMemoryContext, AllocSetContext));
+ planner_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "explain analyze planner context",
+ ALLOCSET_DEFAULT_SIZES);
+ saved_ctx = MemoryContextSwitchTo(planner_ctx);
+ }
if (es->buffers)
bufusage_start = pgBufferUsage;
@@ -624,6 +641,12 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ if (es->memory)
+ {
+ MemoryContextSwitchTo(saved_ctx);
+ MemoryContextMemConsumed(planner_ctx, &mem_counts);
+ }
+
/* calc differences of buffer counters. */
if (es->buffers)
{
@@ -640,7 +663,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
if (pstmt->commandType != CMD_UTILITY)
ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL));
+ &planduration, (es->buffers ? &bufusage : NULL),
+ (es->memory ? &mem_counts : NULL));
else
ExplainOneUtility(pstmt->utilityStmt, into, es, query_string,
paramLI, queryEnv);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6..58634e5ad6 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -687,6 +687,19 @@ MemoryContextMemAllocated(MemoryContext context, bool recurse)
return total;
}
+/*
+ * Return the memory consumption statistics about the given context and its
+ * children.
+ */
+void
+MemoryContextMemConsumed(MemoryContext context,
+ MemoryContextCounters *consumed)
+{
+ memset(consumed, 0, sizeof(*consumed));
+
+ MemoryContextStatsInternal(context, 0, false, 0, consumed, false);
+}
+
/*
* MemoryContextStats
* Print statistics about the named context and all its descendants.
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index f9525fb572..798d1be2de 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -45,6 +45,7 @@ typedef struct ExplainState
bool wal; /* print WAL usage */
bool timing; /* print detailed node timing */
bool summary; /* print total planning and execution timing */
+ bool memory; /* print planner's memory usage information */
bool settings; /* print modified settings */
bool generic; /* generate a generic plan */
ExplainFormat format; /* output format */
@@ -92,7 +93,8 @@ extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
const instr_time *planduration,
- const BufferUsage *bufusage);
+ const BufferUsage *bufusage,
+ const MemoryContextCounters *mem_counts);
extern void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc);
extern void ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc);
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index d14e8546a6..6f2abcfe4e 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -84,6 +84,8 @@ extern Size GetMemoryChunkSpace(void *pointer);
extern MemoryContext MemoryContextGetParent(MemoryContext context);
extern bool MemoryContextIsEmpty(MemoryContext context);
extern Size MemoryContextMemAllocated(MemoryContext context, bool recurse);
+extern void MemoryContextMemConsumed(MemoryContext context,
+ MemoryContextCounters *consumed);
extern void MemoryContextStats(MemoryContext context);
extern void MemoryContextStatsDetail(MemoryContext context, int max_children,
bool print_to_stderr);
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 809655e16e..86bfdfd29e 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -326,6 +326,74 @@ select explain_filter('explain (generic_plan) select unique1 from tenk1 where th
select explain_filter('explain (analyze, generic_plan) select unique1 from tenk1 where thousand = $1');
ERROR: EXPLAIN options ANALYZE and GENERIC_PLAN cannot be used together
CONTEXT: PL/pgSQL function explain_filter(text) line 5 at FOR over EXECUTE statement
+-- MEMORY option
+select explain_filter('explain (memory) select * from int8_tbl i8');
+ explain_filter
+---------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ Planner Memory: used=N bytes allocated=N bytes
+(2 rows)
+
+select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
+ explain_filter
+-----------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
+ Planning Time: N.N ms
+ Planner Memory: used=N bytes allocated=N bytes
+ Execution Time: N.N ms
+(4 rows)
+
+select explain_filter('explain (memory, summary, format yaml) select * from int8_tbl i8');
+ explain_filter
+-------------------------------
+ - Plan: +
+ Node Type: "Seq Scan" +
+ Parallel Aware: false +
+ Async Capable: false +
+ Relation Name: "int8_tbl"+
+ Alias: "i8" +
+ Startup Cost: N.N +
+ Total Cost: N.N +
+ Plan Rows: N +
+ Plan Width: N +
+ Planning Time: N.N +
+ Planner Memory: +
+ Used: N +
+ Allocated: N
+(1 row)
+
+select explain_filter('explain (memory, analyze, format json) select * from int8_tbl i8');
+ explain_filter
+------------------------------------
+ [ +
+ { +
+ "Plan": { +
+ "Node Type": "Seq Scan", +
+ "Parallel Aware": false, +
+ "Async Capable": false, +
+ "Relation Name": "int8_tbl",+
+ "Alias": "i8", +
+ "Startup Cost": N.N, +
+ "Total Cost": N.N, +
+ "Plan Rows": N, +
+ "Plan Width": N, +
+ "Actual Startup Time": N.N, +
+ "Actual Total Time": N.N, +
+ "Actual Rows": N, +
+ "Actual Loops": N +
+ }, +
+ "Planning Time": N.N, +
+ "Planner Memory": { +
+ "Used": N, +
+ "Allocated": N +
+ }, +
+ "Triggers": [ +
+ ], +
+ "Execution Time": N.N +
+ } +
+ ]
+(1 row)
+
-- Test EXPLAIN (GENERIC_PLAN) with partition pruning
-- partitions should be pruned at plan time, based on constants,
-- but there should be no pruning based on parameter placeholders
diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql
index b6b7beab27..f719e92f84 100644
--- a/src/test/regress/sql/explain.sql
+++ b/src/test/regress/sql/explain.sql
@@ -94,6 +94,12 @@ select explain_filter('explain (generic_plan) select unique1 from tenk1 where th
-- should fail
select explain_filter('explain (analyze, generic_plan) select unique1 from tenk1 where thousand = $1');
+-- MEMORY option
+select explain_filter('explain (memory) select * from int8_tbl i8');
+select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
+select explain_filter('explain (memory, summary, format yaml) select * from int8_tbl i8');
+select explain_filter('explain (memory, analyze, format json) select * from int8_tbl i8');
+
-- Test EXPLAIN (GENERIC_PLAN) with partition pruning
-- partitions should be pruned at plan time, based on constants,
-- but there should be no pruning based on parameter placeholders
--
2.25.1
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v24 2/8] Row pattern recognition patch (parse/analysis).
@ 2024-12-19 06:06 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Tatsuo Ishii @ 2024-12-19 06:06 UTC (permalink / raw)
---
src/backend/parser/parse_agg.c | 7 +
src/backend/parser/parse_clause.c | 297 +++++++++++++++++++++++++++++-
src/backend/parser/parse_expr.c | 6 +
src/backend/parser/parse_func.c | 3 +
4 files changed, 312 insertions(+), 1 deletion(-)
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 04b4596a65..6af3bcb375 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -580,6 +580,10 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
+
/*
* There is intentionally no default: case here, so that the
* compiler will warn if we add a new ParseExprKind without
@@ -970,6 +974,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
case EXPR_KIND_CYCLE_MARK:
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
/*
* There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 979926b605..5a44c68b08 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -96,7 +96,14 @@ static WindowClause *findWindowClause(List *wclist, const char *name);
static Node *transformFrameOffset(ParseState *pstate, int frameOptions,
Oid rangeopfamily, Oid rangeopcintype, Oid *inRangeFunc,
Node *clause);
-
+static void transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist);
+static List *transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist);
+static void transformPatternClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef);
+static List *transformMeasureClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef);
/*
* transformFromClause -
@@ -2954,6 +2961,10 @@ transformWindowDefinitions(ParseState *pstate,
rangeopfamily, rangeopcintype,
&wc->endInRangeFunc,
windef->endOffset);
+
+ /* Process Row Pattern Recognition related clauses */
+ transformRPR(pstate, wc, windef, targetlist);
+
wc->winref = winref;
result = lappend(result, wc);
@@ -3821,3 +3832,287 @@ transformFrameOffset(ParseState *pstate, int frameOptions,
return node;
}
+
+/*
+ * transformRPR
+ * Process Row Pattern Recognition related clauses
+ */
+static void
+transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist)
+{
+ /*
+ * Window definition exists?
+ */
+ if (windef == NULL)
+ return;
+
+ /*
+ * Row Pattern Common Syntax clause exists?
+ */
+ if (windef->rpCommonSyntax == NULL)
+ return;
+
+ /* Check Frame option. Frame must start at current row */
+ if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("FRAME must start at current row when row patttern recognition is used")));
+
+ /* Transform AFTER MACH SKIP TO clause */
+ wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo;
+
+ /* Transform AFTER MACH SKIP TO variable */
+ wc->rpSkipVariable = windef->rpCommonSyntax->rpSkipVariable;
+
+ /* Transform SEEK or INITIAL clause */
+ wc->initial = windef->rpCommonSyntax->initial;
+
+ /* Transform DEFINE clause into list of TargetEntry's */
+ wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist);
+
+ /* Check PATTERN clause and copy to patternClause */
+ transformPatternClause(pstate, wc, windef);
+
+ /* Transform MEASURE clause */
+ transformMeasureClause(pstate, wc, windef);
+}
+
+/*
+ * transformDefineClause
+ * Process DEFINE clause and transform ResTarget into list of
+ * TargetEntry.
+ *
+ * XXX we only support column reference in row pattern definition search
+ * condition, e.g. "price". <row pattern definition variable name>.<column
+ * reference> is not supported, e.g. "A.price".
+ */
+static List *
+transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
+ List **targetlist)
+{
+ /* DEFINE variable name initials */
+ static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz";
+
+ ListCell *lc,
+ *l;
+ ResTarget *restarget,
+ *r;
+ List *restargets;
+ List *defineClause;
+ char *name;
+ int initialLen;
+ int i;
+
+ /*
+ * If Row Definition Common Syntax exists, DEFINE clause must exist. (the
+ * raw parser should have already checked it.)
+ */
+ Assert(windef->rpCommonSyntax->rpDefs != NULL);
+
+ /*
+ * Check and add "A AS A IS TRUE" if pattern variable is missing in DEFINE
+ * per the SQL standard.
+ */
+ restargets = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpPatterns)
+ {
+ A_Expr *a;
+ bool found = false;
+
+ if (!IsA(lfirst(lc), A_Expr))
+ ereport(ERROR,
+ errmsg("node type is not A_Expr"));
+
+ a = (A_Expr *) lfirst(lc);
+ name = strVal(a->lexpr);
+
+ foreach(l, windef->rpCommonSyntax->rpDefs)
+ {
+ restarget = (ResTarget *) lfirst(l);
+
+ if (!strcmp(restarget->name, name))
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ /*
+ * "name" is missing. So create "name AS name IS TRUE" ResTarget
+ * node and add it to the temporary list.
+ */
+ A_Const *n;
+
+ restarget = makeNode(ResTarget);
+ n = makeNode(A_Const);
+ n->val.boolval.type = T_Boolean;
+ n->val.boolval.boolval = true;
+ n->location = -1;
+ restarget->name = pstrdup(name);
+ restarget->indirection = NIL;
+ restarget->val = (Node *) n;
+ restarget->location = -1;
+ restargets = lappend((List *) restargets, restarget);
+ }
+ }
+
+ if (list_length(restargets) >= 1)
+ {
+ /* add missing DEFINEs */
+ windef->rpCommonSyntax->rpDefs =
+ list_concat(windef->rpCommonSyntax->rpDefs, restargets);
+ list_free(restargets);
+ }
+
+ /*
+ * Check for duplicate row pattern definition variables. The standard
+ * requires that no two row pattern definition variable names shall be
+ * equivalent.
+ */
+ restargets = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpDefs)
+ {
+ restarget = (ResTarget *) lfirst(lc);
+ name = restarget->name;
+
+ /*
+ * Add DEFINE expression (Restarget->val) to the targetlist as a
+ * TargetEntry if it does not exist yet. Planner will add the column
+ * ref var node to the outer plan's target list later on. This makes
+ * DEFINE expression could access the outer tuple while evaluating
+ * PATTERN.
+ *
+ * XXX: adding whole expressions of DEFINE to the plan.targetlist is
+ * not so good, because it's not necessary to evalute the expression
+ * in the target list while running the plan. We should extract the
+ * var nodes only then add them to the plan.targetlist.
+ */
+ findTargetlistEntrySQL99(pstate, (Node *) restarget->val,
+ targetlist, EXPR_KIND_RPR_DEFINE);
+
+ /*
+ * Make sure that the row pattern definition search condition is a
+ * boolean expression.
+ */
+ transformWhereClause(pstate, restarget->val,
+ EXPR_KIND_RPR_DEFINE, "DEFINE");
+
+ foreach(l, restargets)
+ {
+ char *n;
+
+ r = (ResTarget *) lfirst(l);
+ n = r->name;
+
+ if (!strcmp(n, name))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("row pattern definition variable name \"%s\" appears more than once in DEFINE clause",
+ name),
+ parser_errposition(pstate, exprLocation((Node *) r))));
+ }
+ restargets = lappend(restargets, restarget);
+ }
+ list_free(restargets);
+
+ /*
+ * Create list of row pattern DEFINE variable name's initial. We assign
+ * [a-z] to them (up to 26 variable names are allowed).
+ */
+ restargets = NIL;
+ i = 0;
+ initialLen = strlen(defineVariableInitials);
+
+ foreach(lc, windef->rpCommonSyntax->rpDefs)
+ {
+ char initial[2];
+
+ restarget = (ResTarget *) lfirst(lc);
+ name = restarget->name;
+
+ if (i >= initialLen)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("number of row pattern definition variable names exceeds %d",
+ initialLen),
+ parser_errposition(pstate,
+ exprLocation((Node *) restarget))));
+ }
+ initial[0] = defineVariableInitials[i++];
+ initial[1] = '\0';
+ wc->defineInitial = lappend(wc->defineInitial,
+ makeString(pstrdup(initial)));
+ }
+
+ defineClause = transformTargetList(pstate, windef->rpCommonSyntax->rpDefs,
+ EXPR_KIND_RPR_DEFINE);
+
+ /* mark column origins */
+ markTargetListOrigins(pstate, defineClause);
+
+ /* mark all nodes in the DEFINE clause tree with collation information */
+ assign_expr_collations(pstate, (Node *) defineClause);
+
+ return defineClause;
+}
+
+/*
+ * transformPatternClause
+ * Process PATTERN clause and return PATTERN clause in the raw parse tree
+ */
+static void
+transformPatternClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef)
+{
+ ListCell *lc;
+
+ /*
+ * Row Pattern Common Syntax clause exists?
+ */
+ if (windef->rpCommonSyntax == NULL)
+ return;
+
+ wc->patternVariable = NIL;
+ wc->patternRegexp = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpPatterns)
+ {
+ A_Expr *a;
+ char *name;
+ char *regexp;
+
+ if (!IsA(lfirst(lc), A_Expr))
+ ereport(ERROR,
+ errmsg("node type is not A_Expr"));
+
+ a = (A_Expr *) lfirst(lc);
+ name = strVal(a->lexpr);
+
+ wc->patternVariable = lappend(wc->patternVariable, makeString(pstrdup(name)));
+ regexp = strVal(lfirst(list_head(a->name)));
+
+ wc->patternRegexp = lappend(wc->patternRegexp, makeString(pstrdup(regexp)));
+ }
+}
+
+/*
+ * transformMeasureClause
+ * Process MEASURE clause
+ * XXX MEASURE clause is not supported yet
+ */
+static List *
+transformMeasureClause(ParseState *pstate, WindowClause *wc,
+ WindowDef *windef)
+{
+ if (windef->rowPatternMeasures == NIL)
+ return NIL;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("%s", "MEASURE clause is not supported yet"),
+ parser_errposition(pstate, exprLocation((Node *) windef->rowPatternMeasures))));
+ return NIL;
+}
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index c2806297aa..e827c59fd4 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -575,6 +575,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
case EXPR_KIND_COPY_WHERE:
case EXPR_KIND_GENERATED_COLUMN:
case EXPR_KIND_CYCLE_MARK:
+ case EXPR_KIND_RPR_DEFINE:
/* okay */
break;
@@ -1858,6 +1859,9 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
case EXPR_KIND_GENERATED_COLUMN:
err = _("cannot use subquery in column generation expression");
break;
+ case EXPR_KIND_RPR_DEFINE:
+ err = _("cannot use subquery in DEFINE expression");
+ break;
/*
* There is intentionally no default: case here, so that the
@@ -3197,6 +3201,8 @@ ParseExprKindName(ParseExprKind exprKind)
return "GENERATED AS";
case EXPR_KIND_CYCLE_MARK:
return "CYCLE";
+ case EXPR_KIND_RPR_DEFINE:
+ return "DEFINE";
/*
* There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 9b23344a3b..4c482abb30 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2658,6 +2658,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
case EXPR_KIND_CYCLE_MARK:
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
/*
* There is intentionally no default: case here, so that the
--
2.25.1
----Next_Part(Thu_Dec_19_15_19_50_2024_894)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v24-0003-Row-pattern-recognition-patch-rewriter.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-12-19 06:06 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 22:13 [PATCH v6] fix psql display of FKs Alvaro Herrera <[email protected]>
2023-12-11 13:35 Re: Report planning memory in EXPLAIN ANALYZE Ashutosh Bapat <[email protected]>
2023-12-12 20:11 ` Re: Report planning memory in EXPLAIN ANALYZE Alvaro Herrera <[email protected]>
2023-12-12 20:20 ` Re: Report planning memory in EXPLAIN ANALYZE Alvaro Herrera <[email protected]>
2023-12-13 10:51 ` Re: Report planning memory in EXPLAIN ANALYZE Ashutosh Bapat <[email protected]>
2024-12-19 06:06 [PATCH v24 2/8] Row pattern recognition patch (parse/analysis). Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox