Cannot read floating point infinity values from JDBC

[lukaseder]

Describe the problem

PostgreSQL supports 'Inf' as a valid literal for the floating point special value Infinity, but the JDBC driver can't seem to read it from CockroachDB

To Reproduce

try (PreparedStatement s = connection.prepareStatement("select real 'Inf', double precision 'Inf'")) {
    try (ResultSet rs = s.executeQuery()) {
        while (rs.next()) {
            System.out.println(rs.getFloat(1));
            System.out.println(rs.getDouble(1));
        }
    }
}

Expected behavior

This works as expected in PostgreSQL, producing 2 times Infinity, but it doesn't work on CockorachDB, where I'm getting this error:

Exception in thread "main" org.postgresql.util.PSQLException: Unzulässiger Wert für den Typ float : +Inf.
	at org.postgresql.jdbc.PgResultSet.toFloat(PgResultSet.java:3091)
	at org.postgresql.jdbc.PgResultSet.getFloat(PgResultSet.java:2411)

One could argue the JDBC driver should understand it as well given that I can use the string literal in casts, too, even in PostgreSQL ('Inf'::real). But for interoperability reasons, but as it stands now, I can't seem to read these values from JDBC in CockroachDB, short of reading strings.

I've also created an issue in pgjdbc to suggest supporting parsing Inf: pgjdbc/pgjdbc#2109

Environment:

• CockroachDB version: CockroachDB CCL v21.1.0-alpha.1 (x86_64-unknown-linux-gnu, built 2020/12/08 17:01:49, go1.15.5)
• Server OS: Linux in Docker in Windows
• Client app: JDBC

[blathers-crl]

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

• Join our community slack channel and ask on #cockroachdb.
• Try find someone from here if you know they worked closely on the area and CC them.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[rafiss]

hi @lukaseder! thanks for this report. we'll take a closer look soon.

[rafiss]

We can probably make this change for 21.2 -- looks like Go is able to parse both Inf and Infinity

[stuckinforloop]

I would like to take this issue.

[rafiss]

@neel229 Thanks!

Here are a few steps:

1. The change needs to be made to pkg/sql/sem/tree/datum.go inside of func (d *DDecimal) Format
2. Then you need to update pkg/cmd/generate-binary/main.go. Look for this comment:

		// The Go binary encoding of NaN differs from Postgres by a 1 at the
		// end. Go also uses Inf instead of Infinity (used by Postgres) for text
		// float encodings. These deviations are still correct, and it's not worth
		// special casing them into the code, so they are commented out here.
		//"NaN",
		//"Inf",
		//"-Inf",

Only remove the comments for Inf and -Inf.
3. Normally we'd have to regenerate the encodings, but to make things easier, just add this diff to pkg/sql/pgwire/testdata/encodings.json

diff --git a/pkg/sql/pgwire/testdata/encodings.json b/pkg/sql/pgwire/testdata/encodings.json
index e1827a2d97..ff72c0362a 100644
--- a/pkg/sql/pgwire/testdata/encodings.json
+++ b/pkg/sql/pgwire/testdata/encodings.json
@@ -755,6 +755,20 @@
         "TextAsBinary": [49, 46, 52, 48, 49, 51, 101, 45, 52, 53],
         "Binary": [0, 0, 0, 1]
     },
+    {
+        "SQL": "'Inf'::float8",
+        "Oid": 701,
+        "Text": "Infinity",
+        "TextAsBinary": [73, 110, 102, 105, 110, 105, 116, 121],
+        "Binary": [127, 240, 0, 0, 0, 0, 0, 0]
+    },
+    {
+        "SQL": "'-Inf'::float8",
+        "Oid": 701,
+        "Text": "-Infinity",
+        "TextAsBinary": [45, 73, 110, 102, 105, 110, 105, 116, 121],
+        "Binary": [255, 240, 0, 0, 0, 0, 0, 0]
+    },
     {
         "SQL": "'-000.000'::float8",
         "Oid": 701,

4. Now make sure that TestEncodings passes with: make test PKG=./pkg/sql/pgwire TESTS=TestEncodings

After you do all this, I'm guessing some of our other tests will fail. But start with these and we'll address the other ones later.

[stuckinforloop]

Alright 👍🏻

[stuckinforloop]

I won't be able to give this time but if anyone else wants to go ahead, please do so.

[iAziz786]

Okay, I want to contribute to the CockroachDB since this issue gets available I'm willing to pick it. :)

[iAziz786]

Trying to get my head around the codebase. So far, I have failed miserably but anyway I'm going to try again until this doesn't get fixed.

@rafiss I have done the suggested changes except for the actual implementation which is going to be in datum.go. Thanks for explaining it so simple but need a little more help from you. I'm not able to find where is the actual parsing happening for Inf and Infinity?

In the func (d *DDecimal) Format I can see a bunch of WriteByte calls and one WriteString but can't understand where is the actual parsing lives. Can you please help a bit?

[rafiss]

@iAziz786 thanks for your interest!

for this issue, we are trying to change the formatting logic for floating point values. the original issue report talks about how JDBC cannot parse the value that is sent out by CockroachDB. so the goal here is to change what CockroachDB is outputting; the goal is not to change any parsing logic in CockroachDB.

also, i realized i made a mistake earlier: we need to update func (d *DFloat) Format, no need to change func (d *DDecimal) Format.

to answer your question, in func (d *DFloat) Format we should update this part:

	if _, frac := math.Modf(fl); frac == 0 && -1000000 < *d && *d < 1000000 {
		// d is a small whole number. Ensure it is printed using a decimal point.
		ctx.Printf("%.1f", fl)
	} else {
		ctx.Printf("%g", fl)
	}

add in a few new else if conditions that check for math.IsInf(fl, 1) and math.IsInf(fl, -1) -- make it output either Infinity or -Infinity.