Re: postgres_fdw and skip locked

2022-02-20 Thread Alexander Pyhalov

Ashutosh Bapat писал 2022-02-17 16:30:

On Wed, Feb 16, 2022 at 8:38 PM Alexander Pyhalov
 wrote:


Ashutosh Bapat писал 2022-02-16 16:40:
> On Mon, Feb 14, 2022 at 4:23 PM Alexander Pyhalov
>  wrote:
>>
> I see that these options will work for all kinds of relations. So no
> problem if foreign table is pointing to something other than a table.

Hi.
The issue is that we perform deparsing while planing, we haven't
connected to server yet.
Are there any ideas how to find out its version without specifying it,
for example, in server options?


Yes, you are right. I wish foreign servers, esp. postgresql, could be
more integrated and if we could defer deparsing till execution phase.
But that's out of scope for this patch.


Hi.
I've updated patch to provide server-level "deparse_wait_policy" option, 
which controls if we deparse SKIP LOCKED or SELECT FOR UPDATE (true by 
default).
This will make possible for people with old servers to revert to old 
behavior.

--
Best regards,
Alexander Pyhalov,
Postgres ProfessionalFrom 5f5a0434227debd8ca7ee406e8cb1997edff14a3 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov 
Date: Mon, 14 Feb 2022 12:01:26 +0300
Subject: [PATCH] postgres_fdw could pass lock wait policy to foreign server

---
 contrib/postgres_fdw/deparse.c|  12 +-
 .../postgres_fdw/expected/postgres_fdw.out| 110 +-
 contrib/postgres_fdw/option.c |   4 +-
 contrib/postgres_fdw/postgres_fdw.c   |   4 +
 contrib/postgres_fdw/postgres_fdw.h   |   1 +
 contrib/postgres_fdw/sql/postgres_fdw.sql |  18 +++
 doc/src/sgml/postgres-fdw.sgml|  16 +++
 7 files changed, 161 insertions(+), 4 deletions(-)

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index bf12eac0288..f55afd461f0 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1429,8 +1429,7 @@ deparseLockingClause(deparse_expr_cxt *context)
  * For now, just ignore any [NO] KEY specification, since (a)
  * it's not clear what that means for a remote table that we
  * don't have complete information about, and (b) it wouldn't
- * work anyway on older remote servers.  Likewise, we don't
- * worry about NOWAIT.
+ * work anyway on older remote servers.
  */
 switch (rc->strength)
 {
@@ -1451,6 +1450,15 @@ deparseLockingClause(deparse_expr_cxt *context)
 if (bms_membership(rel->relids) == BMS_MULTIPLE &&
 	rc->strength != LCS_NONE)
 	appendStringInfo(buf, " OF %s%d", REL_ALIAS_PREFIX, relid);
+
+/* Lock behavior */
+if (fpinfo->deparse_wait_policy && rc->strength != LCS_NONE)
+{
+	if (rc->waitPolicy == LockWaitSkip)
+		appendStringInfoString(buf, " SKIP LOCKED");
+	else if (rc->waitPolicy == LockWaitError)
+		appendStringInfoString(buf, " NOWAIT");
+}
 			}
 		}
 	}
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index b2e02caefe4..b33d61b8656 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -389,6 +389,35 @@ SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
  102 |  2 | 00102 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2  | 2  | foo
 (1 row)
 
+-- test wait policy specification
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE NOWAIT;
+   QUERY PLAN
+-
+ Foreign Scan on public.ft1 t1
+   Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 101)) FOR UPDATE NOWAIT
+(3 rows)
+
+SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE NOWAIT;
+ c1  | c2 |  c3   |  c4  |c5| c6 | c7 | c8  
+-++---+--+--+++-
+ 101 |  1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1  | 1  | foo
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE SKIP LOCKED;
+ QUERY PLAN  
+-
+ Foreign Scan on public.ft1 t1
+   Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 102)) FOR SHARE SKIP LOCKED
+(3 rows)
+
+SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE SKIP LOCKED;
+ c1  | c2 |  c3   |  c4  |c5| c6 | c7 | c8  
+-++---+--+--

Re: postgres_fdw and skip locked

2022-02-17 Thread Ashutosh Bapat
On Wed, Feb 16, 2022 at 8:38 PM Alexander Pyhalov
 wrote:
>
> Ashutosh Bapat писал 2022-02-16 16:40:
> > On Mon, Feb 14, 2022 at 4:23 PM Alexander Pyhalov
> >  wrote:
> >>
> >> Hi.
> >>
> >> Now select ... for update ... [skip locked|nowait] options are not
> >> pushed down to remote servers. I see the only reason is that we can
> >> speak to pre-9.5 server, which doesn't understand skip locked option.
> >> Are there any other possible issues? Should we add foreign table
> >> option
> >> to control this behavior?
> >
> > Should we always push these clauses if remote server's version is
> > newer than 9.5? There are quite a few options already. It will be good
> > not to add one more.
> >
> > I see that these options will work for all kinds of relations. So no
> > problem if foreign table is pointing to something other than a table.
>
> Hi.
> The issue is that we perform deparsing while planing, we haven't
> connected to server yet.
> Are there any ideas how to find out its version without specifying it,
> for example, in server options?

Yes, you are right. I wish foreign servers, esp. postgresql, could be
more integrated and if we could defer deparsing till execution phase.
But that's out of scope for this patch.

-- 
Best Wishes,
Ashutosh Bapat




Re: postgres_fdw and skip locked

2022-02-16 Thread Alexander Pyhalov

Ashutosh Bapat писал 2022-02-16 16:40:

On Mon, Feb 14, 2022 at 4:23 PM Alexander Pyhalov
 wrote:


Hi.

Now select ... for update ... [skip locked|nowait] options are not
pushed down to remote servers. I see the only reason is that we can
speak to pre-9.5 server, which doesn't understand skip locked option.
Are there any other possible issues? Should we add foreign table 
option

to control this behavior?


Should we always push these clauses if remote server's version is
newer than 9.5? There are quite a few options already. It will be good
not to add one more.

I see that these options will work for all kinds of relations. So no
problem if foreign table is pointing to something other than a table.


Hi.
The issue is that we perform deparsing while planing, we haven't 
connected to server yet.
Are there any ideas how to find out its version without specifying it, 
for example, in server options?

--
Best regards,
Alexander Pyhalov,
Postgres Professional




Re: postgres_fdw and skip locked

2022-02-16 Thread Ashutosh Bapat
On Mon, Feb 14, 2022 at 4:23 PM Alexander Pyhalov
 wrote:
>
> Hi.
>
> Now select ... for update ... [skip locked|nowait] options are not
> pushed down to remote servers. I see the only reason is that we can
> speak to pre-9.5 server, which doesn't understand skip locked option.
> Are there any other possible issues? Should we add foreign table option
> to control this behavior?

Should we always push these clauses if remote server's version is
newer than 9.5? There are quite a few options already. It will be good
not to add one more.

I see that these options will work for all kinds of relations. So no
problem if foreign table is pointing to something other than a table.

-- 
Best Wishes,
Ashutosh Bapat




postgres_fdw and skip locked

2022-02-14 Thread Alexander Pyhalov

Hi.

Now select ... for update ... [skip locked|nowait] options are not 
pushed down to remote servers. I see the only reason is that we can 
speak to pre-9.5 server, which doesn't understand skip locked option. 
Are there any other possible issues? Should we add foreign table option 
to control this behavior?


--
Best regards,
Alexander Pyhalov,
Postgres ProfessionalFrom f416cb0afdf42b8ab5375e7a3ccab6e41ebb16ab Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov 
Date: Mon, 14 Feb 2022 12:01:26 +0300
Subject: [PATCH] postgres_fdw could pass lock wait policy to foreign server

---
 contrib/postgres_fdw/deparse.c| 12 ++-
 .../postgres_fdw/expected/postgres_fdw.out| 81 +++
 contrib/postgres_fdw/sql/postgres_fdw.sql | 13 +++
 3 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index bf12eac0288..7e320be5e6a 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1429,8 +1429,7 @@ deparseLockingClause(deparse_expr_cxt *context)
  * For now, just ignore any [NO] KEY specification, since (a)
  * it's not clear what that means for a remote table that we
  * don't have complete information about, and (b) it wouldn't
- * work anyway on older remote servers.  Likewise, we don't
- * worry about NOWAIT.
+ * work anyway on older remote servers.
  */
 switch (rc->strength)
 {
@@ -1451,6 +1450,15 @@ deparseLockingClause(deparse_expr_cxt *context)
 if (bms_membership(rel->relids) == BMS_MULTIPLE &&
 	rc->strength != LCS_NONE)
 	appendStringInfo(buf, " OF %s%d", REL_ALIAS_PREFIX, relid);
+
+/* Lock behavior */
+if (rc->strength != LCS_NONE)
+{
+	if (rc->waitPolicy == LockWaitSkip)
+		appendStringInfoString(buf, " SKIP LOCKED");
+	else if (rc->waitPolicy == LockWaitError)
+		appendStringInfoString(buf, " NOWAIT");
+}
 			}
 		}
 	}
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index b2e02caefe4..89564d9aef5 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -389,6 +389,35 @@ SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE;
  102 |  2 | 00102 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2  | 2  | foo
 (1 row)
 
+-- test wait policy specification
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE NOWAIT;
+   QUERY PLAN
+-
+ Foreign Scan on public.ft1 t1
+   Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 101)) FOR UPDATE NOWAIT
+(3 rows)
+
+SELECT * FROM ft1 t1 WHERE c1 = 101 FOR UPDATE NOWAIT;
+ c1  | c2 |  c3   |  c4  |c5| c6 | c7 | c8  
+-++---+--+--+++-
+ 101 |  1 | 00101 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1  | 1  | foo
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE SKIP LOCKED;
+ QUERY PLAN  
+-
+ Foreign Scan on public.ft1 t1
+   Output: c1, c2, c3, c4, c5, c6, c7, c8, t1.*
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = 102)) FOR SHARE SKIP LOCKED
+(3 rows)
+
+SELECT * FROM ft1 t1 WHERE c1 = 102 FOR SHARE SKIP LOCKED;
+ c1  | c2 |  c3   |  c4  |c5| c6 | c7 | c8  
+-++---+--+--+++-
+ 102 |  2 | 00102 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2  | 2  | foo
+(1 row)
+
 -- aggregate
 SELECT COUNT(*) FROM ft1 t1;
  count 
@@ -1862,6 +1891,32 @@ SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t
  110 | 110
 (10 rows)
 
+-- test wait policy specification
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR UPDATE SKIP LOCKED;
+QUERY PLAN