date:20201221

RE: libpq debug log

2020-12-21 Thread k.jami...@fujitsu.com

On Tuesday, December 15, 2020 8:12 PM, Iwata-san wrote:
> > There are some things still to do:
> I worked on some to do.

Hi Iwata-san,

Thank you for updating the patch.
I would recommend to register this patch in the upcoming commitfest
to help us keep track of it. I will follow the thread to provide more
reviews too.

> > 1. Is the handling of protocol 2 correct?
> Protocol 3.0 is implemented in PostgreSQL v7.4 or later. Therefore, most
> servers and clients today want to connect using 3.0.
> Also, wishful thinking, I think Protocol 2.0 will no longer be supported. So I
> didn't develop it aggressively.
> Another reason I'm concerned about implementing it would make the code
> less maintainable.

Though I have also not tested it with 2.0 server yet, do I have to download 7.3
version to test that isn't it? Silly question, do we still want to have this
feature for 2.0?
I understand that protocol 2.0 is still supported, but it is only used for
PostgreSQL versions 7.3 and earlier, which is not updated by fixes anymore
since we only backpatch up to previous 5 versions. However I am not sure if
it's a good idea, but how about if we just support this feature for protocol 3.
There are similar chunks of code in fe-exec.c, PQsendPrepare(), 
PQsendDescribe(),
etc. that already do something similar, so I just thought in hindsight if we can
do the same.
if (PG_PROTOCOL_MAJOR(conn->pversion) >= 3)
{

}
else
{
/* Protocol 2.0 isn't supported */
 printfPQExpBuffer(&conn->errorMessage,
   libpq_gettext("function requires at least protocol 
version 3.0\n"));
 return 0;
}

But if it's necessary to provide this improved trace output for 2.0 servers, 
then ignore what
I suggested above, and although difficult I think we should test for protocol 
2.0 in older server.

Some minor code comments I noticed:
1.
+   LOG_FIRST_BYTE, /* logging the first byte 
identifing the
+* protocol 
message type */

"identifing" should be "identifying". And closing */ should be on 3rd line. 

2.
+   case LOG_CONTENTS:
+   /* Suppresses printing terminating zero byte */

--> Suppress printing of terminating zero byte

Regards,
Kirk Jamison

Re: Commit fest manager for 2021-01

2020-12-21 Thread Magnus Hagander

On Sun, Dec 20, 2020 at 10:57 PM Masahiko Sawada  wrote:
>
> On Sun, Dec 20, 2020 at 10:27 PM Magnus Hagander  wrote:
> >
> > On Sat, Dec 19, 2020 at 6:00 AM Michael Paquier  wrote:
> > >
> > > On Sat, Dec 19, 2020 at 10:03:47AM +0530, Amit Kapila wrote:
> > > > Glad to hear. I am confident that you can do justice to this role.
> > >
> > > I also think you will do just fine.  Thanks for taking care of this.
> >
> > +1 on both accounts.
> >
> > If you haven't been one before (which I think?), please let me know
> > what username your account in the system has, and I will make sure you
> > get the required permissions-
>
> Thanks!
>
> My usename is masahikosawada.

I've now added the required permissions.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

Re: Strange behavior with polygon and NaN

2020-12-21 Thread Kyotaro Horiguchi

At Tue, 01 Dec 2020 10:03:42 -0500, Tom Lane  wrote in 
> I think it should be "needs review" now.

Conflicted with some commit(s) uncertain to me. Rebased.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 26d9edeccf3f3111a7e0ff049aa6e3ba3746dce9 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 13 Nov 2020 13:31:20 +0900
Subject: [PATCH v8] fix geometric nan handling

---
 doc/src/sgml/func.sgml |  17 ++
 src/backend/utils/adt/geo_ops.c| 337 ++---
 src/include/utils/float.h  |  22 ++
 src/test/regress/expected/create_index.out |   2 +-
 src/test/regress/expected/geometry.out | 331 +---
 5 files changed, 485 insertions(+), 224 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d5cd705eeb..2f7f3c633c 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10855,6 +10855,23 @@ CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple
 

 
+   
+ 
+   NaN and Infinity make geometric functions and operators behave
+   inconsistently. Geometric operators or functions that return a boolean
+   return false for operands that contain NaNs. Number-returning functions
+   and operators return NaN in most cases but sometimes return a valid
+   value if no NaNs are met while actual calculation.  Object-returning ones
+   yield an object that contain NaNs depending to the operation.  Likewise
+   the objects containing Infinity can make geometric operators and
+   functions behave inconsistently. For example (point
+   '(Infinity,Infinity)' <-> line '{-1,0,5}') is Infinity but (point
+   '(Infinity,Infinity)' <-> line '{0,-1,5}') is NaN. It can never
+   be a value other than these, but you should consider it uncertain how
+   geometric operators behave for objects containing Infinity.
+ 
+   
+

 Geometric Functions
 
diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c
index c1dc511a1a..e74bf3031e 100644
--- a/src/backend/utils/adt/geo_ops.c
+++ b/src/backend/utils/adt/geo_ops.c
@@ -904,9 +904,9 @@ box_intersect(PG_FUNCTION_ARGS)
 
 	result = (BOX *) palloc(sizeof(BOX));
 
-	result->high.x = float8_min(box1->high.x, box2->high.x);
+	result->high.x = float8_min_nan(box1->high.x, box2->high.x);
 	result->low.x = float8_max(box1->low.x, box2->low.x);
-	result->high.y = float8_min(box1->high.y, box2->high.y);
+	result->high.y = float8_min_nan(box1->high.y, box2->high.y);
 	result->low.y = float8_max(box1->low.y, box2->low.y);
 
 	PG_RETURN_BOX_P(result);
@@ -1061,15 +1061,23 @@ line_construct(LINE *result, Point *pt, float8 m)
 		result->A = -1.0;
 		result->B = 0.0;
 		result->C = pt->x;
+
+		/* Avoid creating a valid line from an invalid point */
+		if (likely(!isnan(pt->x) && !isnan(pt->y)))
+			return;
 	}
-	else if (m == 0)
+	else if (m == 0.0)
 	{
 		/* horizontal - use "y = C" */
 		result->A = 0.0;
 		result->B = -1.0;
 		result->C = pt->y;
+
+		/* Avoid creating a valid line from an invalid point */
+		if (likely(!isnan(pt->x) && !isnan(pt->y)))
+			return;
 	}
-	else
+	else if (!isnan(m))
 	{
 		/* use "mx - y + yinter = 0" */
 		result->A = m;
@@ -1078,7 +1086,12 @@ line_construct(LINE *result, Point *pt, float8 m)
 		/* on some platforms, the preceding expression tends to produce -0 */
 		if (result->C == 0.0)
 			result->C = 0.0;
+
+		if (likely(!isnan(result->C)))
+			return;
 	}
+
+	result->A = result->B = result->C = get_float8_nan();
 }
 
 /* line_construct_pp()
@@ -1091,6 +1104,7 @@ line_construct_pp(PG_FUNCTION_ARGS)
 	Point	   *pt2 = PG_GETARG_POINT_P(1);
 	LINE	   *result = (LINE *) palloc(sizeof(LINE));
 
+	/* NaNs are considered to be equal by point_eq_point */
 	if (point_eq_point(pt1, pt2))
 		ereport(ERROR,
 (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -,8 +1125,12 @@ line_intersect(PG_FUNCTION_ARGS)
 {
 	LINE	   *l1 = PG_GETARG_LINE_P(0);
 	LINE	   *l2 = PG_GETARG_LINE_P(1);
+	Point		xp;
 
-	PG_RETURN_BOOL(line_interpt_line(NULL, l1, l2));
+	if (line_interpt_line(&xp, l1, l2) && !isnan(xp.x) && !isnan(xp.y))
+		PG_RETURN_BOOL(true);
+	else
+		PG_RETURN_BOOL(false);
 }
 
 Datum
@@ -1130,14 +1148,17 @@ line_perp(PG_FUNCTION_ARGS)
 	LINE	   *l1 = PG_GETARG_LINE_P(0);
 	LINE	   *l2 = PG_GETARG_LINE_P(1);
 
+	if (unlikely(isnan(l1->C) || isnan(l2->C)))
+		return false;
+
 	if (FPzero(l1->A))
-		PG_RETURN_BOOL(FPzero(l2->B));
+		PG_RETURN_BOOL(FPzero(l2->B) && !isnan(l1->B) && !isnan(l2->A));
 	if (FPzero(l2->A))
-		PG_RETURN_BOOL(FPzero(l1->B));
+		PG_RETURN_BOOL(FPzero(l1->B) && !isnan(l2->B) && !isnan(l1->A));
 	if (FPzero(l1->B))
-		PG_RETURN_BOOL(FPzero(l2->A));
+		PG_RETURN_BOOL(FPzero(l2->A) && !isnan(l1->A) && !isnan(l2->B));
 	if (FPzero(l2->B))
-		PG_RETURN_BOOL(FPzero(l1->A));
+		PG_RETURN_BOOL(FPzero(l1->A) && !isnan(l2->A) && !isnan(l1->B));
 
 	PG_RETURN_BOOL(FPeq(float8_div(float8_mul(l1->A, l2->A

bad dependency in pg_dump output related to support function breaks binary upgrade

2020-12-21 Thread Pavel Stehule

Hi

some Orafce's user reported problems with pg_upgrade. I checked this issue
and it looks like pg_dump problem:


pg_restore: creating FUNCTION "public.nvarchar2("public"."nvarchar2",
integer, boolean)"
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 612; 1255 33206 FUNCTION
nvarchar2("public"."nvarchar2", integer, boolean) pavel
pg_restore: error: could not execute query: ERROR:  function
public.nvarchar2_transform(internal) does not exist
Command was: CREATE FUNCTION "public"."nvarchar2"("public"."nvarchar2",
integer, boolean) RETURNS "public"."nvarchar2"
LANGUAGE "c" IMMUTABLE STRICT SUPPORT "public"."nvarchar2_transform"
AS '$libdir/orafce', 'nvarchar2';




--
-- TOC entry 612 (class 1255 OID 33206)
-- Name: nvarchar2("public"."nvarchar2", integer, boolean); Type: FUNCTION;
Schema: public; Owner: pavel
--

CREATE FUNCTION "public"."nvarchar2"("public"."nvarchar2", integer,
boolean) RETURNS "public"."nvarchar2"
LANGUAGE "c" IMMUTABLE STRICT SUPPORT "public"."nvarchar2_transform"
AS '$libdir/orafce', 'nvarchar2';

-- For binary upgrade, handle extension membership the hard way
ALTER EXTENSION "orafce" ADD FUNCTION
"public"."nvarchar2"("public"."nvarchar2", integer, boolean);


ALTER FUNCTION "public"."nvarchar2"("public"."nvarchar2", integer, boolean)
OWNER TO "pavel";

--
-- TOC entry 607 (class 1255 OID 33201)
-- Name: nvarchar2_transform("internal"); Type: FUNCTION; Schema: public;
Owner: pavel
--

CREATE FUNCTION "public"."nvarchar2_transform"("internal") RETURNS
"internal"
LANGUAGE "c" IMMUTABLE STRICT
AS '$libdir/orafce', 'orafce_varchar_transform';

-- For binary upgrade, handle extension membership the hard way
ALTER EXTENSION "orafce" ADD FUNCTION
"public"."nvarchar2_transform"("internal");


ALTER FUNCTION "public"."nvarchar2_transform"("internal") OWNER TO "pavel";

the supporting function should be dumped first before function where
supporting function is used.

Regards

Pavel

Re: Single transaction in the tablesync worker?

2020-12-21 Thread Peter Smith

Hi Amit.

PSA my v5 WIP patch for the Solution1.

This patch still applies onto the v30 patch set [1] from other 2PC thread:
[1]
https://www.postgresql.org/message-id/CAFPTHDYA8yE6tEmQ2USYS68kNt%2BkM%3DSwKgj%3Djy4AvFD5e9-UTQ%40mail.gmail.com

(I understand you would like this to be delivered as a separate patch
independent of v30. I will convert it ASAP)

Coded / WIP:

* tablesync slot is now permanent instead of temporary. The tablesync
slot name is no longer tied to the Subscription slot name.

* the tablesync slot cleanup (drop) code is added for DropSubscription
and for finish_sync_worker functions

* tablesync worked now allowing multiple tx instead of single tx

* a new state (SUBREL_STATE_COPYDONE) is persisted after a successful
copy_table in LogicalRepSyncTableStart.

* if a relaunched tablesync finds the state is SUBREL_STATE_COPYDONE
then it will bypass the initial copy_table phase.

* tablesync sets up replication origin tracking in
LogicalRepSyncTableStart (similar as done for the apply worker). The
origin is advanced when first created.

* tablesync replication origin tracking is cleaned up during
DropSubscription and/or process_syncing_tables_for_apply

TODO / Known Issues:

* I think if there are crashed tablesync workers they may not be known
to DropSubscription current code. This might be a problem to cleanup
slots and/or origin tracking belonging to those unknown workers.

70 matches

Mail list logo