public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/3] postgres_fdw: Perform UPPERREL_ORDERED step remotely
14+ messages / 6 participants
[nested] [flat]
* [PATCH 1/3] postgres_fdw: Perform UPPERREL_ORDERED step remotely
@ 2019-03-07 09:27 Etsuro Fujita <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Etsuro Fujita @ 2019-03-07 09:27 UTC (permalink / raw)
---
contrib/postgres_fdw/deparse.c | 28 +-
.../postgres_fdw/expected/postgres_fdw.out | 167 +++-----
contrib/postgres_fdw/postgres_fdw.c | 382 +++++++++++++++++-
contrib/postgres_fdw/postgres_fdw.h | 9 +-
4 files changed, 460 insertions(+), 126 deletions(-)
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 92a0ab6da5..97dd07bee8 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -167,7 +167,8 @@ static void printRemotePlaceholder(Oid paramtype, int32 paramtypmod,
static void deparseSelectSql(List *tlist, bool is_subquery, List **retrieved_attrs,
deparse_expr_cxt *context);
static void deparseLockingClause(deparse_expr_cxt *context);
-static void appendOrderByClause(List *pathkeys, deparse_expr_cxt *context);
+static void appendOrderByClause(List *pathkeys, bool has_final_sort,
+ deparse_expr_cxt *context);
static void appendConditions(List *exprs, deparse_expr_cxt *context);
static void deparseFromExprForRel(StringInfo buf, PlannerInfo *root,
RelOptInfo *foreignrel, bool use_alias,
@@ -929,8 +930,8 @@ build_tlist_to_deparse(RelOptInfo *foreignrel)
void
deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *rel,
List *tlist, List *remote_conds, List *pathkeys,
- bool is_subquery, List **retrieved_attrs,
- List **params_list)
+ bool has_final_sort, bool is_subquery,
+ List **retrieved_attrs, List **params_list)
{
deparse_expr_cxt context;
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) rel->fdw_private;
@@ -985,7 +986,7 @@ deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *rel,
/* Add ORDER BY clause if we found any useful pathkeys */
if (pathkeys)
- appendOrderByClause(pathkeys, &context);
+ appendOrderByClause(pathkeys, has_final_sort, &context);
/* Add any necessary FOR UPDATE/SHARE. */
deparseLockingClause(&context);
@@ -1590,7 +1591,7 @@ deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
/* Deparse the subquery representing the relation. */
appendStringInfoChar(buf, '(');
deparseSelectStmtForRel(buf, root, foreignrel, NIL,
- fpinfo->remote_conds, NIL, true,
+ fpinfo->remote_conds, NIL, false, true,
&retrieved_attrs, params_list);
appendStringInfoChar(buf, ')');
@@ -3109,7 +3110,8 @@ appendGroupByClause(List *tlist, deparse_expr_cxt *context)
* base relation are obtained and deparsed.
*/
static void
-appendOrderByClause(List *pathkeys, deparse_expr_cxt *context)
+appendOrderByClause(List *pathkeys, bool has_final_sort,
+ deparse_expr_cxt *context)
{
ListCell *lcell;
int nestlevel;
@@ -3126,7 +3128,19 @@ appendOrderByClause(List *pathkeys, deparse_expr_cxt *context)
PathKey *pathkey = lfirst(lcell);
Expr *em_expr;
- em_expr = find_em_expr_for_rel(pathkey->pk_eclass, baserel);
+ if (has_final_sort)
+ {
+ /*
+ * By construction, context->foreignrel is the input relation to
+ * the final sort.
+ */
+ em_expr = find_em_expr_for_input_target(context->root,
+ pathkey->pk_eclass,
+ context->foreignrel->reltarget);
+ }
+ else
+ em_expr = find_em_expr_for_rel(pathkey->pk_eclass, baserel);
+
Assert(em_expr != NULL);
appendStringInfoString(buf, delim);
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 42108bd3d4..592dd3e05f 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2552,18 +2552,13 @@ DROP ROLE regress_view_owner;
-- Simple aggregates
explain (verbose, costs off)
select count(c6), sum(c1), avg(c1), min(c2), max(c1), stddev(c2), sum(c1) * (random() <= 1)::int as sum2 from ft1 where c2 < 5 group by c2 order by 1, 2;
- QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------------
- Result
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (count(c6)), (sum(c1)), (avg(c1)), (min(c2)), (max(c1)), (stddev(c2)), ((sum(c1)) * ((random() <= '1'::double precision))::integer), c2
- -> Sort
- Output: (count(c6)), (sum(c1)), (avg(c1)), (min(c2)), (max(c1)), (stddev(c2)), c2
- Sort Key: (count(ft1.c6)), (sum(ft1.c1))
- -> Foreign Scan
- Output: (count(c6)), (sum(c1)), (avg(c1)), (min(c2)), (max(c1)), (stddev(c2)), c2
- Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT count(c6), sum("C 1"), avg("C 1"), min(c2), max("C 1"), stddev(c2), c2 FROM "S 1"."T 1" WHERE ((c2 < 5)) GROUP BY 7
-(9 rows)
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT count(c6), sum("C 1"), avg("C 1"), min(c2), max("C 1"), stddev(c2), c2 FROM "S 1"."T 1" WHERE ((c2 < 5)) GROUP BY 7 ORDER BY count(c6) ASC NULLS LAST, sum("C 1") ASC NULLS LAST
+(4 rows)
select count(c6), sum(c1), avg(c1), min(c2), max(c1), stddev(c2), sum(c1) * (random() <= 1)::int as sum2 from ft1 where c2 < 5 group by c2 order by 1, 2;
count | sum | avg | min | max | stddev | sum2
@@ -2621,16 +2616,13 @@ select sum(t1.c1), count(t2.c1) from ft1 t1 inner join ft2 t2 on (t1.c1 = t2.c1)
-- GROUP BY clause having expressions
explain (verbose, costs off)
select c2/2, sum(c2) * (c2/2) from ft1 group by c2/2 order by c2/2;
- QUERY PLAN
----------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: ((c2 / 2)), ((sum(c2) * (c2 / 2)))
- Sort Key: ((ft1.c2 / 2))
- -> Foreign Scan
- Output: ((c2 / 2)), ((sum(c2) * (c2 / 2)))
- Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT (c2 / 2), (sum(c2) * (c2 / 2)) FROM "S 1"."T 1" GROUP BY 1
-(7 rows)
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT (c2 / 2), (sum(c2) * (c2 / 2)) FROM "S 1"."T 1" GROUP BY 1 ORDER BY (c2 / 2) ASC NULLS LAST
+(4 rows)
select c2/2, sum(c2) * (c2/2) from ft1 group by c2/2 order by c2/2;
?column? | ?column?
@@ -2645,18 +2637,15 @@ select c2/2, sum(c2) * (c2/2) from ft1 group by c2/2 order by c2/2;
-- Aggregates in subquery are pushed down.
explain (verbose, costs off)
select count(x.a), sum(x.a) from (select c2 a, sum(c1) b from ft1 group by c2, sqrt(c1) order by 1, 2) x;
- QUERY PLAN
----------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------------
Aggregate
Output: count(ft1.c2), sum(ft1.c2)
- -> Sort
+ -> Foreign Scan
Output: ft1.c2, (sum(ft1.c1)), (sqrt((ft1.c1)::double precision))
- Sort Key: ft1.c2, (sum(ft1.c1))
- -> Foreign Scan
- Output: ft1.c2, (sum(ft1.c1)), (sqrt((ft1.c1)::double precision))
- Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, sum("C 1"), sqrt("C 1") FROM "S 1"."T 1" GROUP BY 1, 3
-(9 rows)
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT c2, sum("C 1"), sqrt("C 1") FROM "S 1"."T 1" GROUP BY 1, 3 ORDER BY c2 ASC NULLS LAST, sum("C 1") ASC NULLS LAST
+(6 rows)
select count(x.a), sum(x.a) from (select c2 a, sum(c1) b from ft1 group by c2, sqrt(c1) order by 1, 2) x;
count | sum
@@ -2742,16 +2731,13 @@ select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2
-- Also, ORDER BY contains an aggregate function
explain (verbose, costs off)
select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
- QUERY PLAN
------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: c2, c2, (sum(c1))
- Sort Key: (sum(ft1.c1))
- -> Foreign Scan
- Output: c2, c2, (sum(c1))
- Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, c2, sum("C 1") FROM "S 1"."T 1" WHERE ((c2 > 6)) GROUP BY 1, 2
-(7 rows)
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT c2, c2, sum("C 1") FROM "S 1"."T 1" WHERE ((c2 > 6)) GROUP BY 1, 2 ORDER BY sum("C 1") ASC NULLS LAST
+(4 rows)
select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
c2 | c2
@@ -2764,16 +2750,13 @@ select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
-- Testing HAVING clause shippability
explain (verbose, costs off)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
- QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: c2, (sum(c1))
- Sort Key: ft2.c2
- -> Foreign Scan
- Output: c2, (sum(c1))
- Relations: Aggregate on (public.ft2)
- Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY 1 HAVING ((avg("C 1") < 500::numeric)) AND ((sum("C 1") < 49800))
-(7 rows)
+ Relations: Aggregate on (public.ft2)
+ Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY 1 HAVING ((avg("C 1") < 500::numeric)) AND ((sum("C 1") < 49800)) ORDER BY c2 ASC NULLS LAST
+(4 rows)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
c2 | sum
@@ -2823,16 +2806,13 @@ select sum(c1) from ft1 group by c2 having avg(c1 * (random() <= 1)::int) > 100
-- ORDER BY within aggregate, same column used to order
explain (verbose, costs off)
select array_agg(c1 order by c1) from ft1 where c1 < 100 group by c2 order by 1;
- QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (array_agg(c1 ORDER BY c1)), c2
- Sort Key: (array_agg(ft1.c1 ORDER BY ft1.c1))
- -> Foreign Scan
- Output: (array_agg(c1 ORDER BY c1)), c2
- Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT array_agg("C 1" ORDER BY "C 1" ASC NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) GROUP BY 2
-(7 rows)
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT array_agg("C 1" ORDER BY "C 1" ASC NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) GROUP BY 2 ORDER BY array_agg("C 1" ORDER BY "C 1" ASC NULLS LAST) ASC NULLS LAST
+(4 rows)
select array_agg(c1 order by c1) from ft1 where c1 < 100 group by c2 order by 1;
array_agg
@@ -2869,16 +2849,13 @@ select array_agg(c5 order by c1 desc) from ft2 where c2 = 6 and c1 < 50;
-- DISTINCT within aggregate
explain (verbose, costs off)
select array_agg(distinct (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (array_agg(DISTINCT (t1.c1 % 5))), ((t2.c1 % 3))
- Sort Key: (array_agg(DISTINCT (t1.c1 % 5)))
- -> Foreign Scan
- Output: (array_agg(DISTINCT (t1.c1 % 5))), ((t2.c1 % 3))
- Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5)), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2
-(7 rows)
+ Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
+ Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5)), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2 ORDER BY array_agg(DISTINCT (r1.c1 % 5)) ASC NULLS LAST
+(4 rows)
select array_agg(distinct (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
array_agg
@@ -2890,16 +2867,13 @@ select array_agg(distinct (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2
-- DISTINCT combined with ORDER BY within aggregate
explain (verbose, costs off)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
- QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5))), ((t2.c1 % 3))
- Sort Key: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5)))
- -> Foreign Scan
- Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5))), ((t2.c1 % 3))
- Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) ASC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2
-(7 rows)
+ Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
+ Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) ASC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2 ORDER BY array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) ASC NULLS LAST) ASC NULLS LAST
+(4 rows)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
array_agg
@@ -2910,16 +2884,13 @@ select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5) from ft4 t1 full join ft
explain (verbose, costs off)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5 desc nulls last) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5) DESC NULLS LAST)), ((t2.c1 % 3))
- Sort Key: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5) DESC NULLS LAST))
- -> Foreign Scan
- Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5) DESC NULLS LAST)), ((t2.c1 % 3))
- Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) DESC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2
-(7 rows)
+ Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
+ Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) DESC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2 ORDER BY array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) DESC NULLS LAST) ASC NULLS LAST
+(4 rows)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5 desc nulls last) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
array_agg
@@ -2931,16 +2902,13 @@ select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5 desc nulls last) from ft4
-- FILTER within aggregate
explain (verbose, costs off)
select sum(c1) filter (where c1 < 100 and c2 > 5) from ft1 group by c2 order by 1 nulls last;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (sum(c1) FILTER (WHERE ((c1 < 100) AND (c2 > 5)))), c2
- Sort Key: (sum(ft1.c1) FILTER (WHERE ((ft1.c1 < 100) AND (ft1.c2 > 5))))
- -> Foreign Scan
- Output: (sum(c1) FILTER (WHERE ((c1 < 100) AND (c2 > 5)))), c2
- Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT sum("C 1") FILTER (WHERE (("C 1" < 100) AND (c2 > 5))), c2 FROM "S 1"."T 1" GROUP BY 2
-(7 rows)
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT sum("C 1") FILTER (WHERE (("C 1" < 100) AND (c2 > 5))), c2 FROM "S 1"."T 1" GROUP BY 2 ORDER BY sum("C 1") FILTER (WHERE (("C 1" < 100) AND (c2 > 5))) ASC NULLS LAST
+(4 rows)
select sum(c1) filter (where c1 < 100 and c2 > 5) from ft1 group by c2 order by 1 nulls last;
sum
@@ -3339,16 +3307,13 @@ select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x w
-- FULL join with IS NULL check in HAVING
explain (verbose, costs off)
select avg(t1.c1), sum(t2.c1) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) group by t2.c1 having (avg(t1.c1) is null and sum(t2.c1) < 10) or sum(t2.c1) is null order by 1 nulls last, 2;
- QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- Sort
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan
Output: (avg(t1.c1)), (sum(t2.c1)), t2.c1
- Sort Key: (avg(t1.c1)), (sum(t2.c1))
- -> Foreign Scan
- Output: (avg(t1.c1)), (sum(t2.c1)), t2.c1
- Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT avg(r1.c1), sum(r2.c1), r2.c1 FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) GROUP BY 3 HAVING ((((avg(r1.c1) IS NULL) AND (sum(r2.c1) < 10)) OR (sum(r2.c1) IS NULL)))
-(7 rows)
+ Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
+ Remote SQL: SELECT avg(r1.c1), sum(r2.c1), r2.c1 FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) GROUP BY 3 HAVING ((((avg(r1.c1) IS NULL) AND (sum(r2.c1) < 10)) OR (sum(r2.c1) IS NULL))) ORDER BY avg(r1.c1) ASC NULLS LAST, sum(r2.c1) ASC NULLS LAST
+(4 rows)
select avg(t1.c1), sum(t2.c1) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) group by t2.c1 having (avg(t1.c1) is null and sum(t2.c1) < 10) or sum(t2.c1) is null order by 1 nulls last, 2;
avg | sum
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 2f387fac42..c342bd319c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -246,6 +246,25 @@ typedef struct PgFdwAnalyzeState
MemoryContext temp_cxt; /* context for per-tuple temporary data */
} PgFdwAnalyzeState;
+/*
+ * This enum describes what's kept in the fdw_private list for a ForeignPath.
+ * We store:
+ *
+ * 1) Boolean flag showing if the remote query has the final sort
+ */
+enum FdwPathPrivateIndex
+{
+ /* has-final-sort flag (as an integer Value node) */
+ FdwPathPrivateHasFinalSort
+};
+
+/* Struct for extra information passed to estimate_path_cost_size */
+typedef struct
+{
+ PathTarget *target;
+ bool has_final_sort;
+} PgFdwPathExtraData;
+
/*
* Identify the attribute where data conversion fails.
*/
@@ -368,6 +387,7 @@ static void estimate_path_cost_size(PlannerInfo *root,
RelOptInfo *foreignrel,
List *param_join_conds,
List *pathkeys,
+ PgFdwPathExtraData *fpextra,
double *p_rows, int *p_width,
Cost *p_startup_cost, Cost *p_total_cost);
static void get_remote_estimate(const char *sql,
@@ -376,6 +396,12 @@ static void get_remote_estimate(const char *sql,
int *width,
Cost *startup_cost,
Cost *total_cost);
+static void adjust_foreign_grouping_path_cost(PlannerInfo *root,
+ List *pathkeys,
+ double retrieved_rows,
+ double width,
+ Cost *p_startup_cost,
+ Cost *p_run_cost);
static bool ec_member_matches_foreign(PlannerInfo *root, RelOptInfo *rel,
EquivalenceClass *ec, EquivalenceMember *em,
void *arg);
@@ -452,6 +478,9 @@ static void add_foreign_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
RelOptInfo *grouped_rel,
GroupPathExtraData *extra);
+static void add_foreign_ordered_paths(PlannerInfo *root,
+ RelOptInfo *input_rel,
+ RelOptInfo *ordered_rel);
static void apply_server_options(PgFdwRelationInfo *fpinfo);
static void apply_table_options(PgFdwRelationInfo *fpinfo);
static void merge_fdw_options(PgFdwRelationInfo *fpinfo,
@@ -637,7 +666,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
* values in fpinfo so we don't need to do it again to generate the
* basic foreign path.
*/
- estimate_path_cost_size(root, baserel, NIL, NIL,
+ estimate_path_cost_size(root, baserel, NIL, NIL, NULL,
&fpinfo->rows, &fpinfo->width,
&fpinfo->startup_cost, &fpinfo->total_cost);
@@ -668,7 +697,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
set_baserel_size_estimates(root, baserel);
/* Fill in basically-bogus cost estimates for use later. */
- estimate_path_cost_size(root, baserel, NIL, NIL,
+ estimate_path_cost_size(root, baserel, NIL, NIL, NULL,
&fpinfo->rows, &fpinfo->width,
&fpinfo->startup_cost, &fpinfo->total_cost);
}
@@ -1102,7 +1131,7 @@ postgresGetForeignPaths(PlannerInfo *root,
/* Get a cost estimate from the remote */
estimate_path_cost_size(root, baserel,
- param_info->ppi_clauses, NIL,
+ param_info->ppi_clauses, NIL, NULL,
&rows, &width,
&startup_cost, &total_cost);
@@ -1149,8 +1178,16 @@ postgresGetForeignPlan(PlannerInfo *root,
List *fdw_recheck_quals = NIL;
List *retrieved_attrs;
StringInfoData sql;
+ bool has_final_sort = false;
ListCell *lc;
+ /*
+ * Get private info created by postgresGetForeignUpperPaths, if any.
+ */
+ if (best_path->fdw_private)
+ has_final_sort = intVal(list_nth(best_path->fdw_private,
+ FdwPathPrivateHasFinalSort));
+
if (IS_SIMPLE_REL(foreignrel))
{
/*
@@ -1299,7 +1336,8 @@ postgresGetForeignPlan(PlannerInfo *root,
initStringInfo(&sql);
deparseSelectStmtForRel(&sql, root, foreignrel, fdw_scan_tlist,
remote_exprs, best_path->path.pathkeys,
- false, &retrieved_attrs, ¶ms_list);
+ has_final_sort, false,
+ &retrieved_attrs, ¶ms_list);
/* Remember remote_exprs for possible use by postgresPlanDirectModify */
fpinfo->final_remote_exprs = remote_exprs;
@@ -2482,6 +2520,8 @@ postgresExplainDirectModify(ForeignScanState *node, ExplainState *es)
*
* param_join_conds are the parameterization clauses with outer relations.
* pathkeys specify the expected sort order if any for given path being costed.
+ * fpextra specifies additional post-scan/join processing steps such as the
+ * final sort.
*
* The function returns the cost and size estimates in p_row, p_width,
* p_startup_cost and p_total_cost variables.
@@ -2491,6 +2531,7 @@ estimate_path_cost_size(PlannerInfo *root,
RelOptInfo *foreignrel,
List *param_join_conds,
List *pathkeys,
+ PgFdwPathExtraData *fpextra,
double *p_rows, int *p_width,
Cost *p_startup_cost, Cost *p_total_cost)
{
@@ -2555,8 +2596,9 @@ estimate_path_cost_size(PlannerInfo *root,
initStringInfo(&sql);
appendStringInfoString(&sql, "EXPLAIN ");
deparseSelectStmtForRel(&sql, root, foreignrel, fdw_scan_tlist,
- remote_conds, pathkeys, false,
- &retrieved_attrs, NULL);
+ remote_conds, pathkeys,
+ fpextra ? fpextra->has_final_sort : false,
+ false, &retrieved_attrs, NULL);
/* Get the remote estimate */
conn = GetConnection(fpinfo->user, false);
@@ -2632,6 +2674,19 @@ estimate_path_cost_size(PlannerInfo *root,
{
startup_cost = fpinfo->rel_startup_cost;
run_cost = fpinfo->rel_total_cost - fpinfo->rel_startup_cost;
+
+ /*
+ * When we're called from postgresGetForeignUpperPaths() with the
+ * UPPERREL_ORDERED stage, the costs of scanning a foreign base or
+ * join relation obtained from the cache would not yet contain the
+ * eval costs for the final scan/join target that has been updated
+ * by apply_scanjoin_target_to_paths(); add the eval costs now.
+ */
+ if (fpextra && !IS_UPPER_REL(foreignrel))
+ {
+ startup_cost += foreignrel->reltarget->cost.startup;
+ run_cost += foreignrel->reltarget->cost.per_tuple * rows;
+ }
}
else if (IS_JOIN_REL(foreignrel))
{
@@ -2844,13 +2899,39 @@ estimate_path_cost_size(PlannerInfo *root,
*/
if (pathkeys != NIL)
{
- startup_cost *= DEFAULT_FDW_SORT_MULTIPLIER;
- run_cost *= DEFAULT_FDW_SORT_MULTIPLIER;
+ if (IS_UPPER_REL(foreignrel))
+ {
+ Assert(fpinfo->stage == UPPERREL_GROUP_AGG);
+ adjust_foreign_grouping_path_cost(root, pathkeys,
+ retrieved_rows, width,
+ &startup_cost, &run_cost);
+ }
+ else
+ {
+ startup_cost *= DEFAULT_FDW_SORT_MULTIPLIER;
+ run_cost *= DEFAULT_FDW_SORT_MULTIPLIER;
+ }
}
total_cost = startup_cost + run_cost;
}
+ /*
+ * If this includes an UPPERREL_ORDERED step, the given target, which will
+ * be applied to the resulting path, might have different expressions from
+ * the foreignrel's reltarget (see make_sort_input_target()); adjust tlist
+ * eval costs.
+ */
+ if (fpextra && fpextra->target != foreignrel->reltarget)
+ {
+ QualCost oldcost = foreignrel->reltarget->cost;
+ QualCost newcost = fpextra->target->cost;
+
+ startup_cost += newcost.startup - oldcost.startup;
+ total_cost += newcost.startup - oldcost.startup;
+ total_cost += (newcost.per_tuple - oldcost.per_tuple) * rows;
+ }
+
/*
* Cache the costs for scans without any pathkeys or parameterization
* before adding the costs for transferring data from the foreign server.
@@ -2860,7 +2941,7 @@ estimate_path_cost_size(PlannerInfo *root,
* foreign server. This function will be called at least once for every
* foreign relation without pathkeys and parameterization.
*/
- if (pathkeys == NIL && param_join_conds == NIL)
+ if (pathkeys == NIL && param_join_conds == NIL && fpextra == NULL)
{
fpinfo->rel_startup_cost = startup_cost;
fpinfo->rel_total_cost = total_cost;
@@ -2935,6 +3016,57 @@ get_remote_estimate(const char *sql, PGconn *conn,
PG_END_TRY();
}
+/*
+ * Adjust the cost estimates of a foreign grouping path.
+ */
+static void
+adjust_foreign_grouping_path_cost(PlannerInfo *root,
+ List *pathkeys,
+ double retrieved_rows,
+ double width,
+ Cost *p_startup_cost,
+ Cost *p_run_cost)
+{
+ /*
+ * If the GROUP BY clause isn't sort-able, the plan chosen by the remote
+ * side is unlikely to generate properly-sorted output, so it would need
+ * an explicit sort; adjust the given costs with cost_sort(). Likewise,
+ * if the GROUP BY clause is sort-able but isn't a superset of the given
+ * pathkeys, adjust the costs with that function. Otherwise, adjust the
+ * costs by applying the same heuristic as for the scan/join case.
+ */
+ if (!grouping_is_sortable(root->parse->groupClause) ||
+ !pathkeys_contained_in(pathkeys, root->group_pathkeys))
+ {
+ Path sort_path; /* dummy for result of cost_sort */
+
+ cost_sort(&sort_path,
+ root,
+ pathkeys,
+ *p_startup_cost + *p_run_cost,
+ retrieved_rows,
+ width,
+ 0.0,
+ work_mem,
+ -1.0);
+
+ *p_startup_cost = sort_path.startup_cost;
+ *p_run_cost = sort_path.total_cost - sort_path.startup_cost;
+ }
+ else
+ {
+ /*
+ * The default cost seems too large for foreign-grouping cases; add
+ * 1/4th of the default.
+ */
+ double sort_multiplier = 1.0 + (DEFAULT_FDW_SORT_MULTIPLIER
+ - 1.0) * 0.25;
+
+ *p_startup_cost *= sort_multiplier;
+ *p_run_cost *= sort_multiplier;
+ }
+}
+
/*
* Detect whether we want to process an EquivalenceClass member.
*
@@ -4934,7 +5066,7 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
List *useful_pathkeys = lfirst(lc);
Path *sorted_epq_path;
- estimate_path_cost_size(root, rel, NIL, useful_pathkeys,
+ estimate_path_cost_size(root, rel, NIL, useful_pathkeys, NULL,
&rows, &width, &startup_cost, &total_cost);
/*
@@ -5185,8 +5317,8 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
extra->sjinfo);
/* Estimate costs for bare join relation */
- estimate_path_cost_size(root, joinrel, NIL, NIL, &rows,
- &width, &startup_cost, &total_cost);
+ estimate_path_cost_size(root, joinrel, NIL, NIL, NULL,
+ &rows, &width, &startup_cost, &total_cost);
/* Now update this information in the joinrel */
joinrel->rows = rows;
joinrel->reltarget->width = width;
@@ -5455,15 +5587,29 @@ postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
return;
/* Ignore stages we don't support; and skip any duplicate calls. */
- if (stage != UPPERREL_GROUP_AGG || output_rel->fdw_private)
+ if ((stage != UPPERREL_GROUP_AGG &&
+ stage != UPPERREL_ORDERED) ||
+ output_rel->fdw_private)
return;
fpinfo = (PgFdwRelationInfo *) palloc0(sizeof(PgFdwRelationInfo));
+ fpinfo->stage = stage;
fpinfo->pushdown_safe = false;
output_rel->fdw_private = fpinfo;
- add_foreign_grouping_paths(root, input_rel, output_rel,
- (GroupPathExtraData *) extra);
+ switch (stage)
+ {
+ case UPPERREL_GROUP_AGG:
+ add_foreign_grouping_paths(root, input_rel, output_rel,
+ (GroupPathExtraData *) extra);
+ break;
+ case UPPERREL_ORDERED:
+ add_foreign_ordered_paths(root, input_rel, output_rel);
+ break;
+ default:
+ elog(ERROR, "unexpected upper relation: %d", (int) stage);
+ break;
+ }
}
/*
@@ -5533,8 +5679,8 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
cost_qual_eval(&fpinfo->local_conds_cost, fpinfo->local_conds, root);
/* Estimate the cost of push down */
- estimate_path_cost_size(root, grouped_rel, NIL, NIL, &rows,
- &width, &startup_cost, &total_cost);
+ estimate_path_cost_size(root, grouped_rel, NIL, NIL, NULL,
+ &rows, &width, &startup_cost, &total_cost);
/* Now update this information in the fpinfo */
fpinfo->rows = rows;
@@ -5542,6 +5688,8 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
fpinfo->startup_cost = startup_cost;
fpinfo->total_cost = total_cost;
+ grouped_rel->rows = fpinfo->rows;
+
/* Create and add foreign path to the grouping relation. */
grouppath = create_foreign_upper_path(root,
grouped_rel,
@@ -5557,6 +5705,144 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
add_path(grouped_rel, (Path *) grouppath);
}
+/*
+ * add_foreign_ordered_paths
+ * Add foreign paths for performing the final sort remotely.
+ *
+ * Given input_rel contains the source-data Paths. The paths are added to the
+ * given ordered_rel.
+ */
+static void
+add_foreign_ordered_paths(PlannerInfo *root, RelOptInfo *input_rel,
+ RelOptInfo *ordered_rel)
+{
+ Query *parse = root->parse;
+ PgFdwRelationInfo *ifpinfo = input_rel->fdw_private;
+ PgFdwRelationInfo *fpinfo = ordered_rel->fdw_private;
+ PgFdwPathExtraData *fpextra;
+ double rows;
+ int width;
+ Cost startup_cost;
+ Cost total_cost;
+ List *fdw_private;
+ ForeignPath *ordered_path;
+ ListCell *lc;
+
+ /* Shouldn't get here unless the query has ORDER BY */
+ Assert(parse->sortClause);
+
+ /*
+ * For now we don't support cases where there are any SRFs in the tlist
+ */
+ if (parse->hasTargetSRFs)
+ return;
+
+ /* Save the input_rel as outerrel in fpinfo */
+ fpinfo->outerrel = input_rel;
+
+ /*
+ * Copy foreign table, foreign server, user mapping, FDW options etc.
+ * details from the input relation's fpinfo.
+ */
+ fpinfo->table = ifpinfo->table;
+ fpinfo->server = ifpinfo->server;
+ fpinfo->user = ifpinfo->user;
+ merge_fdw_options(fpinfo, ifpinfo, NULL);
+
+ /*
+ * No work if there is a properly-sorted ForeignPath in the ordered_rel's
+ * pathlist.
+ */
+ foreach(lc, ordered_rel->pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+
+ /*
+ * apply_scanjoin_target_to_paths() might put a ProjectionPath on top
+ * of each of its scan/join paths; look through ProjectionPath and see
+ * if the path underneath it is ForeignPath.
+ */
+ if (IsA(path, ForeignPath) ||
+ (IsA(path, ProjectionPath) &&
+ IsA(((ProjectionPath *) path)->subpath, ForeignPath)))
+ {
+ Assert(pathkeys_contained_in(root->sort_pathkeys,
+ path->pathkeys));
+
+ /* Safe to push down */
+ fpinfo->pushdown_safe = true;
+
+ return;
+ }
+ }
+
+ /*
+ * We try to create paths below by extending paths for the underlying
+ * base, join, or grouping relation to perform the final sort remotely,
+ * which is stored into the fdw_private list of the resulting paths.
+ */
+
+ /* Assess if it is safe to push down the final sort */
+ foreach(lc, root->sort_pathkeys)
+ {
+ PathKey *pathkey = (PathKey *) lfirst(lc);
+ EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
+ Expr *sort_expr;
+
+ /*
+ * is_foreign_expr would detect volatile expressions as well, but
+ * checking ec_has_volatile here saves some cycles.
+ */
+ if (pathkey_ec->ec_has_volatile)
+ return;
+
+ /* Get the sort expression for the pathkey_ec */
+ sort_expr = find_em_expr_for_input_target(root,
+ pathkey_ec,
+ input_rel->reltarget);
+
+ /* If it's unsafe to remote, we cannot push down the final sort */
+ if (!is_foreign_expr(root, input_rel, sort_expr))
+ return;
+ }
+
+ /* Safe to push down */
+ fpinfo->pushdown_safe = true;
+
+ /* Initialize the selectivity and cost of local_conds */
+ fpinfo->local_conds_sel = 1.0;
+ fpinfo->local_conds_cost.startup = 0.0;
+ fpinfo->local_conds_cost.per_tuple = 0.0;
+
+ fpextra = (PgFdwPathExtraData *) palloc0(sizeof(PgFdwPathExtraData));
+ fpextra->target = root->upper_targets[UPPERREL_ORDERED];
+ fpextra->has_final_sort = true;
+
+ /* Estimate the costs of performing the final sort remotely */
+ estimate_path_cost_size(root, input_rel, NIL, root->sort_pathkeys, fpextra,
+ &rows, &width, &startup_cost, &total_cost);
+
+ /*
+ * Build the fdw_private list that will be used by postgresGetForeignPlan.
+ * Items in the list must match order in enum FdwPathPrivateIndex.
+ */
+ fdw_private = list_make1(makeInteger(true));
+
+ /* Create foreign ordering ForeignPath */
+ ordered_path = create_foreign_upper_path(root,
+ input_rel,
+ root->upper_targets[UPPERREL_ORDERED],
+ rows,
+ startup_cost,
+ total_cost,
+ root->sort_pathkeys,
+ NULL, /* no extra plan */
+ fdw_private);
+
+ /* And add it to the ordered_rel */
+ add_path(ordered_rel, (Path *) ordered_path);
+}
+
/*
* Create a tuple from the specified row of the PGresult.
*
@@ -5807,3 +6093,65 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
/* We didn't find any suitable equivalence class expression */
return NULL;
}
+
+/*
+ * Find an equivalence class member expression to be computed as a sort column
+ * in the given target.
+ */
+Expr *
+find_em_expr_for_input_target(PlannerInfo *root,
+ EquivalenceClass *ec,
+ PathTarget *target)
+{
+ ListCell *lc1;
+ int i;
+
+ i = 0;
+ foreach(lc1, target->exprs)
+ {
+ Expr *expr = (Expr *) lfirst(lc1);
+ Index sgref = get_pathtarget_sortgroupref(target, i);
+ ListCell *lc2;
+
+ /* Ignore non-sort expressions */
+ if (sgref == 0 ||
+ get_sortgroupref_clause_noerr(sgref,
+ root->parse->sortClause) == NULL)
+ {
+ i++;
+ continue;
+ }
+
+ /* We ignore binary-compatible relabeling on both ends */
+ while (expr && IsA(expr, RelabelType))
+ expr = ((RelabelType *) expr)->arg;
+
+ /* Locate an EquivalenceClass member matching this expr, if any */
+ foreach(lc2, ec->ec_members)
+ {
+ EquivalenceMember *em = (EquivalenceMember *) lfirst(lc2);
+ Expr *em_expr;
+
+ /* Don't match constants */
+ if (em->em_is_const)
+ continue;
+
+ /* Ignore child members */
+ if (em->em_is_child)
+ continue;
+
+ /* Match if same expression (after stripping relabel) */
+ em_expr = em->em_expr;
+ while (em_expr && IsA(em_expr, RelabelType))
+ em_expr = ((RelabelType *) em_expr)->arg;
+
+ if (equal(em_expr, expr))
+ return em->em_expr;
+ }
+
+ i++;
+ }
+
+ elog(ERROR, "could not find pathkey item to sort");
+ return NULL; /* keep compiler quiet */
+}
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 3f50103285..936e3f498a 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -92,6 +92,9 @@ typedef struct PgFdwRelationInfo
/* joinclauses contains only JOIN/ON conditions for an outer join */
List *joinclauses; /* List of RestrictInfo */
+ /* Upper relation information */
+ UpperRelationKind stage;
+
/* Grouping information */
List *grouped_tlist;
@@ -175,10 +178,14 @@ extern void deparseAnalyzeSql(StringInfo buf, Relation rel,
List **retrieved_attrs);
extern void deparseStringLiteral(StringInfo buf, const char *val);
extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel);
+extern Expr *find_em_expr_for_input_target(PlannerInfo *root,
+ EquivalenceClass *ec,
+ PathTarget *target);
extern List *build_tlist_to_deparse(RelOptInfo *foreignrel);
extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root,
RelOptInfo *foreignrel, List *tlist,
- List *remote_conds, List *pathkeys, bool is_subquery,
+ List *remote_conds, List *pathkeys,
+ bool has_final_sort, bool is_subquery,
List **retrieved_attrs, List **params_list);
extern const char *get_jointype_name(JoinType jointype);
--
2.19.2
--------------050108040401020008040409
Content-Type: text/x-patch;
name="v6-0002-Refactor-create_limit_path-to-share-cost-adjustment-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="v6-0002-Refactor-create_limit_path-to-share-cost-adjustment-";
filename*1=".patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 7/7] Add a comment to ATExecSetTableSpace.
@ 2019-03-25 11:39 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Kyotaro Horiguchi @ 2019-03-25 11:39 UTC (permalink / raw)
We use heap_register_sync() stuff to control WAL-logging and file sync
on bulk insertion, but we cannot use it because the function lacks the
ability to handle forks explicitly. Add a comment to explain that.
---
src/backend/commands/tablecmds.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 242311b0d7..c7c7bcb308 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11594,7 +11594,13 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
RelationInvalidateWALRequirements(rel);
RelationCreateStorage(newrnode, rel->rd_rel->relpersistence);
- /* copy main fork */
+ /*
+ * copy main fork
+ *
+ * You might think that we could use heap_register_sync() to control file
+ * sync and WAL-logging, but we cannot because the sutff lacks the ability
+ * to handle each fork explicitly.
+ */
copy_relation_data(rel->rd_smgr, dstrel, MAIN_FORKNUM,
rel->rd_rel->relpersistence);
--
2.16.3
----Next_Part(Mon_Mar_25_21_32_04_2019_838)----
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v13 4/8] Row pattern recognition patch (planner).
@ 2024-01-22 09:45 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Tatsuo Ishii @ 2024-01-22 09:45 UTC (permalink / raw)
---
src/backend/optimizer/plan/createplan.c | 23 ++++++++++++++-----
src/backend/optimizer/plan/planner.c | 3 +++
src/backend/optimizer/plan/setrefs.c | 27 ++++++++++++++++++++++-
src/backend/optimizer/prep/prepjointree.c | 4 ++++
src/include/nodes/plannodes.h | 16 ++++++++++++++
5 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index ca619eab94..3c7fd3867b 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -286,9 +286,10 @@ static WindowAgg *make_windowagg(List *tlist, Index winref,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
int frameOptions, Node *startOffset, Node *endOffset,
Oid startInRangeFunc, Oid endInRangeFunc,
- Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst,
- List *runCondition, List *qual, bool topWindow,
- Plan *lefttree);
+ Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, List *runCondition,
+ RPSkipTo rpSkipTo, List *patternVariable, List *patternRegexp, List *defineClause,
+ List *defineInitial,
+ List *qual, bool topWindow, Plan *lefttree);
static Group *make_group(List *tlist, List *qual, int numGroupCols,
AttrNumber *grpColIdx, Oid *grpOperators, Oid *grpCollations,
Plan *lefttree);
@@ -2698,6 +2699,11 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path)
wc->inRangeAsc,
wc->inRangeNullsFirst,
wc->runCondition,
+ wc->rpSkipTo,
+ wc->patternVariable,
+ wc->patternRegexp,
+ wc->defineClause,
+ wc->defineInitial,
best_path->qual,
best_path->topwindow,
subplan);
@@ -6601,8 +6607,10 @@ make_windowagg(List *tlist, Index winref,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
int frameOptions, Node *startOffset, Node *endOffset,
Oid startInRangeFunc, Oid endInRangeFunc,
- Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst,
- List *runCondition, List *qual, bool topWindow, Plan *lefttree)
+ Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, List *runCondition,
+ RPSkipTo rpSkipTo, List *patternVariable, List *patternRegexp, List *defineClause,
+ List *defineInitial,
+ List *qual, bool topWindow, Plan *lefttree)
{
WindowAgg *node = makeNode(WindowAgg);
Plan *plan = &node->plan;
@@ -6628,6 +6636,11 @@ make_windowagg(List *tlist, Index winref,
node->inRangeAsc = inRangeAsc;
node->inRangeNullsFirst = inRangeNullsFirst;
node->topWindow = topWindow;
+ node->rpSkipTo = rpSkipTo,
+ node->patternVariable = patternVariable;
+ node->patternRegexp = patternRegexp;
+ node->defineClause = defineClause;
+ node->defineInitial = defineInitial;
plan->targetlist = tlist;
plan->lefttree = lefttree;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 2e2458b128..98fdfb06e3 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -868,6 +868,9 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
wc->runCondition = (List *) preprocess_expression(root,
(Node *) wc->runCondition,
EXPRKIND_TARGET);
+ wc->defineClause = (List *) preprocess_expression(root,
+ (Node *) wc->defineClause,
+ EXPRKIND_TARGET);
}
parse->limitOffset = preprocess_expression(root, parse->limitOffset,
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 22a1fa29f3..d96bf6d3a0 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -211,7 +211,6 @@ static List *set_windowagg_runcondition_references(PlannerInfo *root,
List *runcondition,
Plan *plan);
-
/*****************************************************************************
*
* SUBPLAN REFERENCES
@@ -2456,6 +2455,32 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset)
NRM_EQUAL,
NUM_EXEC_QUAL(plan));
+ /*
+ * Modifies an expression tree in each DEFINE clause so that all Var
+ * nodes's varno refers to OUTER_VAR.
+ */
+ if (IsA(plan, WindowAgg))
+ {
+ WindowAgg *wplan = (WindowAgg *) plan;
+
+ if (wplan->defineClause != NIL)
+ {
+ foreach(l, wplan->defineClause)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(l);
+
+ tle->expr = (Expr *)
+ fix_upper_expr(root,
+ (Node *) tle->expr,
+ subplan_itlist,
+ OUTER_VAR,
+ rtoffset,
+ NRM_EQUAL,
+ NUM_EXEC_QUAL(plan));
+ }
+ }
+ }
+
pfree(subplan_itlist);
}
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index aa83dd3636..95d7399ab4 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2129,6 +2129,10 @@ perform_pullup_replace_vars(PlannerInfo *root,
if (wc->runCondition != NIL)
wc->runCondition = (List *)
pullup_replace_vars((Node *) wc->runCondition, rvcontext);
+
+ if (wc->defineClause != NIL)
+ wc->defineClause = (List *)
+ pullup_replace_vars((Node *) wc->defineClause, rvcontext);
}
if (parse->onConflict)
{
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index b4ef6bc44c..1b928e5a49 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -20,6 +20,7 @@
#include "lib/stringinfo.h"
#include "nodes/bitmapset.h"
#include "nodes/lockoptions.h"
+#include "nodes/parsenodes.h"
#include "nodes/primnodes.h"
@@ -1096,6 +1097,21 @@ typedef struct WindowAgg
/* nulls sort first for in_range tests? */
bool inRangeNullsFirst;
+ /* Row Pattern Recognition AFTER MACH SKIP clause */
+ RPSkipTo rpSkipTo; /* Row Pattern Skip To type */
+
+ /* Row Pattern PATTERN variable name (list of String) */
+ List *patternVariable;
+
+ /* Row Pattern RPATTERN regular expression quantifier ('+' or ''. list of String) */
+ List *patternRegexp;
+
+ /* Row Pattern DEFINE clause (list of TargetEntry) */
+ List *defineClause;
+
+ /* Row Pattern DEFINE variable initial names (list of String) */
+ List *defineInitial;
+
/*
* false for all apart from the WindowAgg that's closest to the root of
* the plan
--
2.25.1
----Next_Part(Mon_Jan_22_19_26_18_2024_011)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v13-0005-Row-pattern-recognition-patch-executor.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
@ 2024-12-16 21:57 Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Michael Banck @ 2024-12-16 21:57 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi Michael,
On Tue, Dec 10, 2024 at 04:38:24PM +0900, Michael Paquier wrote:
> On Thu, Nov 28, 2024 at 09:17:23PM +0100, Michael Banck wrote:
> > On Tue, Nov 19, 2024 at 07:47:58PM -0500, Greg Sabino Mullane wrote:
> > > Compiled and tested: works fine, so +1 from me. Honestly, I was surprised
> > > %s was still available. :)
> >
> > Thanks. Was that full review? You kept the commitfest item in "Needs
> > Review" state.
>
> No objections to this proposal, that can be useful. No objections
> with the use of 's' for the shortcut in the psql prompt.
Cool.
> Back to your patch, you are missing two things at quick glance:
> - Documentation for the new libpq API in 0001.
> - Documentation for the new SERVICE and its new %s (see section called
> "Prompting" on the psql page.
>
> So please make sure to provide these with the next version of the
> patch.
Thanks, I have added the documentation now in v2.
Michael
Attachments:
[text/x-diff] v2-0001-Add-PQservice-to-PGConn.patch (3.9K, ../../[email protected]/2-v2-0001-Add-PQservice-to-PGConn.patch)
download | inline diff:
From 666b80297f1cb918230b5104e2e8dce08b711394 Mon Sep 17 00:00:00 2001
From: Michael Banck <[email protected]>
Date: Thu, 31 Oct 2024 18:27:52 +0100
Subject: [PATCH v2 1/2] Add PQservice to PGConn.
This adds the content of the database service (if any) to PGConn. One
use for this would be for psql to display the service as part of the
prompt. It also adds PGservice() as a new connection status unction.
---
doc/src/sgml/libpq.sgml | 20 ++++++++++++++++++++
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 11 ++++++++++-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 1 +
5 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 01f259fd0d..105b22b317 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2530,6 +2530,26 @@ char *PQport(const PGconn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQservice">
+ <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Returns the service of the active connection.
+
+<synopsis>
+char *PQservice(const PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the
+ <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
+ Otherwise, if there was no service provided, it returns an empty string.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQtty">
<term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 5d8213e0b5..2ad2cbf5ca 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
+PQservice 206
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index aaf87e8e88..ddcc7b60ab 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -190,7 +190,8 @@ typedef struct _internalPQconninfoOption
static const internalPQconninfoOption PQconninfoOptions[] = {
{"service", "PGSERVICE", NULL, NULL,
- "Database-Service", "", 20, -1},
+ "Database-Service", "", 20,
+ offsetof(struct pg_conn, pgservice)},
{"user", "PGUSER", NULL, NULL,
"Database-User", "", 20,
@@ -7040,6 +7041,14 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
+char *
+PQservice(const PGconn *conn)
+{
+ if (!conn)
+ return NULL;
+ return conn->pgservice;
+}
+
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 15012c770c..5947e7c766 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -385,6 +385,7 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
+extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 08cc391cbd..dcebca9898 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -394,6 +394,7 @@ struct pg_conn
char *fbappname; /* fallback application name */
char *dbName; /* database name */
char *replication; /* connect as the replication standby? */
+ char *pgservice; /* Postgres service, if any */
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
--
2.39.5
[text/x-diff] v2-0002-Add-support-for-database-service-to-psql-prompt.patch (3.2K, ../../[email protected]/3-v2-0002-Add-support-for-database-service-to-psql-prompt.patch)
download | inline diff:
From 5faa624871e3267a780d794e9ad2e9f42fd42311 Mon Sep 17 00:00:00 2001
From: Michael Banck <[email protected]>
Date: Thu, 31 Oct 2024 18:49:14 +0100
Subject: [PATCH v2 2/2] Add support for database service to psql prompt.
This adds a new psql variable SERVICE as well as the string '%s' for the
psql PROMPT, displaying the service (from PGSERVICEFILE) if so desired.
---
doc/src/sgml/ref/psql-ref.sgml | 14 ++++++++++++++
src/bin/psql/command.c | 2 ++
src/bin/psql/prompt.c | 6 ++++++
3 files changed, 22 insertions(+)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e42073ed74..af78e0e87b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4380,6 +4380,15 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-service">
+ <term><varname>SERVICE</varname></term>
+ <listitem>
+ <para>
+ The service from <filename>pg_service.conf</filename>, if applicable.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-shell-error">
<term><varname>SHELL_ERROR</varname></term>
<listitem>
@@ -4685,6 +4694,11 @@ testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
(tilde) if the database is your default database.</para></listitem>
</varlistentry>
+ <varlistentry id="app-psql-prompting-s">
+ <term><literal>%s</literal></term>
+ <listitem><para>The name of the service entry, if any.</para></listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-prompting-numbersign">
<term><literal>%#</literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 1f3cbb11f7..cd16f27947 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4082,6 +4082,7 @@ SyncVariables(void)
pset.sversion = PQserverVersion(pset.db);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
+ SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
@@ -4115,6 +4116,7 @@ void
UnsyncVariables(void)
{
SetVariable(pset.vars, "DBNAME", NULL);
+ SetVariable(pset.vars, "SERVICE", NULL);
SetVariable(pset.vars, "USER", NULL);
SetVariable(pset.vars, "HOST", NULL);
SetVariable(pset.vars, "PORT", NULL);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 0d99d00ac9..de3d976103 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -33,6 +33,7 @@
* %p - backend pid
* %> - database server port number
* %n - database user name
+ * %s - service
* %/ - current database
* %~ - like %/ but "~" when database name equals user name
* %w - whitespace of the same width as the most recent output of PROMPT1
@@ -246,6 +247,11 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
}
break;
+ case 's':
+ if (PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
+
case 'l':
snprintf(buf, sizeof(buf), UINT64_FORMAT, pset.stmt_lineno);
break;
--
2.39.5
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
@ 2024-12-17 07:43 ` Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Michael Paquier @ 2024-12-17 07:43 UTC (permalink / raw)
To: Michael Banck <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Dec 16, 2024 at 10:57:49PM +0100, Michael Banck wrote:
> Thanks, I have added the documentation now in v2.
The doc additions seem fine to me. I've just grabbed three tiny nits,
nothing critical.
+ case 's':
+ if (PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
Other code paths of get_prompt check for pset.db being NULL. True
that it does not matter when calling PQservice() with a connection
that does not exist. For consistency with the surroundings this
should be done at least?
+ <para>
+ The service from <filename>pg_service.conf</filename>, if applicable.
+ </para>
pg_service.conf is not especially true as it depends on the
environment used. For psql, perhaps just use "The service name, if
applicable". No need to be fancy.
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
[...]
+PQservice 206
You didn't miss that, nice.
+ <varlistentry id="app-psql-prompting-s">
+ <term><literal>%s</literal></term>
+ <listitem><para>The name of the service entry, if any.</para></listitem>
+ </varlistentry>
Other entries don't use "if any", would just cut it.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
@ 2024-12-17 08:42 ` Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Michael Banck @ 2024-12-17 08:42 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Tue, Dec 17, 2024 at 04:43:21PM +0900, Michael Paquier wrote:
> On Mon, Dec 16, 2024 at 10:57:49PM +0100, Michael Banck wrote:
> > Thanks, I have added the documentation now in v2.
>
> The doc additions seem fine to me. I've just grabbed three tiny nits,
> nothing critical.
Thanks for the further review.
> + case 's':
> + if (PQservice(pset.db))
> + strlcpy(buf, PQservice(pset.db), sizeof(buf));
> + break;
>
> Other code paths of get_prompt check for pset.db being NULL. True
> that it does not matter when calling PQservice() with a connection
> that does not exist. For consistency with the surroundings this
> should be done at least?
Ok, I've done that.
>
> + <para>
> + The service from <filename>pg_service.conf</filename>, if applicable.
> + </para>
>
> pg_service.conf is not especially true as it depends on the
> environment used. For psql, perhaps just use "The service name, if
> applicable". No need to be fancy.
Done.
> + <varlistentry id="app-psql-prompting-s">
> + <term><literal>%s</literal></term>
> + <listitem><para>The name of the service entry, if any.</para></listitem>
> + </varlistentry>
>
> Other entries don't use "if any", would just cut it.
Done.
V3 attached.
Michael
Attachments:
[text/x-diff] v3-0001-Add-PQservice-to-PGConn.patch (3.9K, ../../[email protected]/2-v3-0001-Add-PQservice-to-PGConn.patch)
download | inline diff:
From 1e71982edce8744f48906d2857d22727691b7267 Mon Sep 17 00:00:00 2001
From: Michael Banck <[email protected]>
Date: Thu, 31 Oct 2024 18:27:52 +0100
Subject: [PATCH v3 1/2] Add PQservice to PGConn.
This adds the content of the database service (if any) to PGConn. One
use for this would be for psql to display the service as part of the
prompt. It also adds PGservice() as a new connection status unction.
---
doc/src/sgml/libpq.sgml | 20 ++++++++++++++++++++
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 11 ++++++++++-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 1 +
5 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 01f259fd0d..105b22b317 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2530,6 +2530,26 @@ char *PQport(const PGconn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQservice">
+ <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Returns the service of the active connection.
+
+<synopsis>
+char *PQservice(const PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the
+ <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
+ Otherwise, if there was no service provided, it returns an empty string.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQtty">
<term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 5d8213e0b5..2ad2cbf5ca 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
+PQservice 206
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index aaf87e8e88..ddcc7b60ab 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -190,7 +190,8 @@ typedef struct _internalPQconninfoOption
static const internalPQconninfoOption PQconninfoOptions[] = {
{"service", "PGSERVICE", NULL, NULL,
- "Database-Service", "", 20, -1},
+ "Database-Service", "", 20,
+ offsetof(struct pg_conn, pgservice)},
{"user", "PGUSER", NULL, NULL,
"Database-User", "", 20,
@@ -7040,6 +7041,14 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
+char *
+PQservice(const PGconn *conn)
+{
+ if (!conn)
+ return NULL;
+ return conn->pgservice;
+}
+
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 15012c770c..5947e7c766 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -385,6 +385,7 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
+extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 08cc391cbd..dcebca9898 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -394,6 +394,7 @@ struct pg_conn
char *fbappname; /* fallback application name */
char *dbName; /* database name */
char *replication; /* connect as the replication standby? */
+ char *pgservice; /* Postgres service, if any */
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
--
2.39.5
[text/x-diff] v3-0002-Add-support-for-database-service-to-psql-prompt.patch (3.2K, ../../[email protected]/3-v3-0002-Add-support-for-database-service-to-psql-prompt.patch)
download | inline diff:
From f70fc781adf0833ecd148d672f22d6a1d860c365 Mon Sep 17 00:00:00 2001
From: Michael Banck <[email protected]>
Date: Thu, 31 Oct 2024 18:49:14 +0100
Subject: [PATCH v3 2/2] Add support for database service to psql prompt.
This adds a new psql variable SERVICE as well as the string '%s' for the
psql PROMPT, displaying the service (from PGSERVICEFILE) if so desired.
---
doc/src/sgml/ref/psql-ref.sgml | 14 ++++++++++++++
src/bin/psql/command.c | 2 ++
src/bin/psql/prompt.c | 6 ++++++
3 files changed, 22 insertions(+)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e42073ed74..6ba85d9acf 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4380,6 +4380,15 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-service">
+ <term><varname>SERVICE</varname></term>
+ <listitem>
+ <para>
+ The service name, if applicable.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-shell-error">
<term><varname>SHELL_ERROR</varname></term>
<listitem>
@@ -4685,6 +4694,11 @@ testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
(tilde) if the database is your default database.</para></listitem>
</varlistentry>
+ <varlistentry id="app-psql-prompting-s">
+ <term><literal>%s</literal></term>
+ <listitem><para>The name of the service entry.</para></listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-prompting-numbersign">
<term><literal>%#</literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 1f3cbb11f7..cd16f27947 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4082,6 +4082,7 @@ SyncVariables(void)
pset.sversion = PQserverVersion(pset.db);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
+ SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
@@ -4115,6 +4116,7 @@ void
UnsyncVariables(void)
{
SetVariable(pset.vars, "DBNAME", NULL);
+ SetVariable(pset.vars, "SERVICE", NULL);
SetVariable(pset.vars, "USER", NULL);
SetVariable(pset.vars, "HOST", NULL);
SetVariable(pset.vars, "PORT", NULL);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 0d99d00ac9..3b3f079229 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -33,6 +33,7 @@
* %p - backend pid
* %> - database server port number
* %n - database user name
+ * %s - service
* %/ - current database
* %~ - like %/ but "~" when database name equals user name
* %w - whitespace of the same width as the most recent output of PROMPT1
@@ -246,6 +247,11 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
}
break;
+ case 's':
+ if (pset.db && PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
+
case 'l':
snprintf(buf, sizeof(buf), UINT64_FORMAT, pset.stmt_lineno);
break;
--
2.39.5
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
@ 2024-12-18 06:17 ` Michael Paquier <[email protected]>
2024-12-19 12:36 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Michael Paquier @ 2024-12-18 06:17 UTC (permalink / raw)
To: Michael Banck <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Dec 17, 2024 at 09:42:36AM +0100, Michael Banck wrote:
> Done.
>
> V3 attached.
Done.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
@ 2024-12-19 12:36 ` Michael Banck <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Michael Banck @ 2024-12-19 12:36 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Wed, Dec 18, 2024 at 03:17:36PM +0900, Michael Paquier wrote:
> On Tue, Dec 17, 2024 at 09:42:36AM +0100, Michael Banck wrote:
> > Done.
> >
> > V3 attached.
>
> Done.
Thanks!
Michael
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
@ 2025-07-06 16:13 ` Noah Misch <[email protected]>
2025-07-07 02:06 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Noah Misch @ 2025-07-06 16:13 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Michael Banck <[email protected]>; Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Dec 18, 2024 at 03:17:36PM +0900, Michael Paquier wrote:
> Done.
This new PQservice() function from commit 4b99fed75 came up in the annual
exports.txt diff. The standard in libpq has been to not clutter the API with
new functions that simply retrieve one PQconninfoOption value. PQconninfo()
provides access to all those values in a generic way. What do you think of
making psql use PQconninfo() for this, then removing PQservice()? The rest of
the commit (adding the struct field, necessary for PQconninfo() to include the
value) looks good.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
@ 2025-07-07 02:06 ` Michael Paquier <[email protected]>
2025-07-07 03:00 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Michael Paquier @ 2025-07-07 02:06 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Michael Banck <[email protected]>; Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sun, Jul 06, 2025 at 09:13:19AM -0700, Noah Misch wrote:
> This new PQservice() function from commit 4b99fed75 came up in the annual
> exports.txt diff. The standard in libpq has been to not clutter the API with
> new functions that simply retrieve one PQconninfoOption value. PQconninfo()
> provides access to all those values in a generic way. What do you think of
> making psql use PQconninfo() for this, then removing PQservice()? The rest of
> the commit (adding the struct field, necessary for PQconninfo() to include the
> value) looks good.
Sure, I was not aware of such a policy. Relying on PQconninfoOption
is less efficient because we would need to look through the whole set
of options when looking for the service name, and this needs one extra
allocation as PQconninfoFree() frees the values allocated. With two
callers perhaps this inefficiency is OK to live with anyway.
What do you think about the attached, then?
--
Michael
Attachments:
[text/x-diff] pqservice-removal.patch (5.7K, ../../[email protected]/2-pqservice-removal.patch)
download | inline diff:
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9fcd2db83265..c839634d4233 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4480,6 +4480,7 @@ SyncVariables(void)
{
char vbuf[32];
const char *server_version;
+ char *service_name;
/* get stuff from connection */
pset.encoding = PQclientEncoding(pset.db);
@@ -4489,12 +4490,16 @@ SyncVariables(void)
setFmtEncoding(pset.encoding);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
- SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding));
+ service_name = get_service_name();
+ SetVariable(pset.vars, "SERVICE", service_name);
+ if (service_name)
+ pg_free(service_name);
+
/* this bit should match connection_warnings(): */
/* Try to get full text form of version, might include "devel" etc */
server_version = PQparameterStatus(pset.db, "server_version");
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index d2c0a49c46c0..3593c0468310 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -2531,6 +2531,41 @@ session_username(void)
return PQuser(pset.db);
}
+/*
+ * Return the service name of the current connection.
+ *
+ * The caller is responsible for freeing the result value allocated.
+ */
+char *
+get_service_name(void)
+{
+ PQconninfoOption *opts;
+ PQconninfoOption *serviceopt = NULL;
+ char *res = NULL;
+
+ if (pset.db == NULL)
+ return NULL;
+
+ opts = PQconninfo(pset.db);
+ if (opts == NULL)
+ return NULL;
+
+ for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
+ {
+ if (strcmp(opt->keyword, "service") == 0)
+ {
+ serviceopt = opt;
+ continue;
+ }
+ }
+
+ /* Take a copy of the value, as it is freed by PQconninfoFree(). */
+ if (serviceopt && serviceopt->val != NULL)
+ res = pg_strdup(serviceopt->val);
+ PQconninfoFree(opts);
+
+ return res;
+}
/* expand_tilde
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 7f1a23de1e82..261199df3187 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -39,6 +39,7 @@ extern bool SendQuery(const char *query);
extern bool is_superuser(void);
extern bool standard_strings(void);
extern const char *session_username(void);
+extern char *get_service_name(void);
extern void expand_tilde(char **filename);
extern void clean_extended_state(void);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 3aa7d2d06c80..cdae9b681500 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -169,8 +169,15 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
break;
/* service name */
case 's':
- if (pset.db && PQservice(pset.db))
- strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ {
+ char *service_name = get_service_name();
+
+ if (service_name)
+ {
+ strlcpy(buf, service_name, sizeof(buf));
+ pg_free(service_name);
+ }
+ }
break;
/* backend pid */
case 'p':
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 0625cf39e9af..dbbae642d769 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,9 +205,8 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
-PQservice 206
-PQsetAuthDataHook 207
-PQgetAuthDataHook 208
-PQdefaultAuthDataHook 209
-PQfullProtocolVersion 210
-appendPQExpBufferVA 211
+PQsetAuthDataHook 206
+PQgetAuthDataHook 207
+PQdefaultAuthDataHook 208
+PQfullProtocolVersion 209
+appendPQExpBufferVA 210
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 51a9c4165845..09eb79812ac6 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -7461,14 +7461,6 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
-char *
-PQservice(const PGconn *conn)
-{
- if (!conn)
- return NULL;
- return conn->pgservice;
-}
-
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 7d3a9df6fd55..af8004f952a5 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -400,7 +400,6 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
-extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 298c4b38ef90..b2c2cf9eac83 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2740,26 +2740,6 @@ char *PQport(const PGconn *conn);
</listitem>
</varlistentry>
- <varlistentry id="libpq-PQservice">
- <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term>
-
- <listitem>
- <para>
- Returns the service of the active connection.
-
-<synopsis>
-char *PQservice(const PGconn *conn);
-</synopsis>
- </para>
-
- <para>
- <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the
- <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
- Otherwise, if there was no service provided, it returns an empty string.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry id="libpq-PQtty">
<term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
[application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
download
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 02:06 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
@ 2025-07-07 03:00 ` Noah Misch <[email protected]>
2025-07-07 23:52 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Noah Misch @ 2025-07-07 03:00 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Michael Banck <[email protected]>; Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Jul 07, 2025 at 11:06:06AM +0900, Michael Paquier wrote:
> On Sun, Jul 06, 2025 at 09:13:19AM -0700, Noah Misch wrote:
> > This new PQservice() function from commit 4b99fed75 came up in the annual
> > exports.txt diff. The standard in libpq has been to not clutter the API with
> > new functions that simply retrieve one PQconninfoOption value. PQconninfo()
> > provides access to all those values in a generic way. What do you think of
> > making psql use PQconninfo() for this, then removing PQservice()? The rest of
> > the commit (adding the struct field, necessary for PQconninfo() to include the
> > value) looks good.
>
> Sure, I was not aware of such a policy. Relying on PQconninfoOption
> is less efficient because we would need to look through the whole set
> of options when looking for the service name, and this needs one extra
> allocation as PQconninfoFree() frees the values allocated. With two
> callers perhaps this inefficiency is OK to live with anyway.
I think the choice to make there is whether to call PQconninfo() once per
prompt emission or to cache the value, invalidating that cache e.g. once per
SyncVariables(). My first thought was to cache, but the decision is not too
important. A PQconninfo() call is likely negligible relative to all that
happens between prompts. Even if not negligible, the overhead of not caching
will affect only prompts using the new escape sequence.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 02:06 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-07 03:00 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
@ 2025-07-07 23:52 ` Michael Paquier <[email protected]>
2025-07-09 00:41 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Michael Paquier @ 2025-07-07 23:52 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Michael Banck <[email protected]>; Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sun, Jul 06, 2025 at 08:00:09PM -0700, Noah Misch wrote:
> I think the choice to make there is whether to call PQconninfo() once per
> prompt emission or to cache the value, invalidating that cache e.g. once per
> SyncVariables(). My first thought was to cache, but the decision is not too
> important. A PQconninfo() call is likely negligible relative to all that
> happens between prompts. Even if not negligible, the overhead of not caching
> will affect only prompts using the new escape sequence.
SyncVariables() happens at startup and when re-syncing a connection
during a check, so that does not really worry me.
By the way, there is a second change in the CF that's suggesting the
addition of a SERVICEFILE variable, which also uses a separate libpq
API (forgot about this one):
https://commitfest.postgresql.org/patch/5387/
Changing the patch on the other thread to use a conninfo is
stright-forward. How about extending the get_service_name()@common.c
I've proposed upthread so as it takes a string in input and it could
be reused for more connection options than only "service" so as it
could be reused there as well?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 02:06 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-07 03:00 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 23:52 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
@ 2025-07-09 00:41 ` Noah Misch <[email protected]>
2025-07-09 04:29 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Noah Misch @ 2025-07-09 00:41 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Michael Banck <[email protected]>; Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jul 08, 2025 at 08:52:08AM +0900, Michael Paquier wrote:
> On Sun, Jul 06, 2025 at 08:00:09PM -0700, Noah Misch wrote:
> > I think the choice to make there is whether to call PQconninfo() once per
> > prompt emission or to cache the value, invalidating that cache e.g. once per
> > SyncVariables(). My first thought was to cache, but the decision is not too
> > important. A PQconninfo() call is likely negligible relative to all that
> > happens between prompts. Even if not negligible, the overhead of not caching
> > will affect only prompts using the new escape sequence.
>
> SyncVariables() happens at startup and when re-syncing a connection
> during a check, so that does not really worry me.
>
> By the way, there is a second change in the CF that's suggesting the
> addition of a SERVICEFILE variable, which also uses a separate libpq
> API (forgot about this one):
> https://commitfest.postgresql.org/patch/5387/
>
> Changing the patch on the other thread to use a conninfo is
> stright-forward. How about extending the get_service_name()@common.c
> I've proposed upthread so as it takes a string in input and it could
> be reused for more connection options than only "service" so as it
> could be reused there as well?
I'd prefer not to get involved in decisions affecting only psql efficiency and
psql code cosmetics. Please make that decision without my input.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [PATCH] Add support for displaying database service in psql prompt
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 02:06 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-07 03:00 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 23:52 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-09 00:41 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
@ 2025-07-09 04:29 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Michael Paquier @ 2025-07-09 04:29 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Michael Banck <[email protected]>; Greg Sabino Mullane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jul 08, 2025 at 05:41:32PM -0700, Noah Misch wrote:
> I'd prefer not to get involved in decisions affecting only psql efficiency and
> psql code cosmetics. Please make that decision without my input.
Okay, I have used this more extensible routine then, planning to use
it for the other patch. The prompt shortcut retrieves the value using
a GetVariable(), rather than looking at the connection options all the
time.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2025-07-09 04:29 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 09:27 [PATCH 1/3] postgres_fdw: Perform UPPERREL_ORDERED step remotely Etsuro Fujita <[email protected]>
2019-03-25 11:39 [PATCH 7/7] Add a comment to ATExecSetTableSpace. Kyotaro Horiguchi <[email protected]>
2024-01-22 09:45 [PATCH v13 4/8] Row pattern recognition patch (planner). Tatsuo Ishii <[email protected]>
2024-12-16 21:57 Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-17 07:43 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-17 08:42 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2024-12-18 06:17 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2024-12-19 12:36 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Banck <[email protected]>
2025-07-06 16:13 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 02:06 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-07 03:00 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-07 23:52 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[email protected]>
2025-07-09 00:41 ` Re: [PATCH] Add support for displaying database service in psql prompt Noah Misch <[email protected]>
2025-07-09 04:29 ` Re: [PATCH] Add support for displaying database service in psql prompt Michael Paquier <[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