NUMERIC type binary decode error

[auntyellow]

Describe the issue
NUMERIC type binary decode error

Driver Version?
42.2.18

Java Version?
1.8

OS Version?
Windows

PostgreSQL Version?
12.0

To Reproduce

public class TestBigDecimal {
	private static Set<Integer> supportedBinaryOids;

	static {
		try {
			Field supportedBinaryOidsField = Class
					.forName("org.postgresql.jdbc.PgConnection")
					.getDeclaredField("SUPPORTED_BINARY_OIDS");
			supportedBinaryOidsField.setAccessible(true);
			supportedBinaryOids = (Set<Integer>) supportedBinaryOidsField.get(null);
		} catch (ReflectiveOperationException e) {
			throw new RuntimeException(e);
		}
	}

	public static void main(String[] args) throws Exception {
		supportedBinaryOids.add(1700);
		Properties p = new Properties();
		p.setProperty("user", "postgres");
		p.setProperty("password", "****");
		p.setProperty("prepareThreshold", "-1");
		try (
			Connection conn = DriverManager.getConnection("jdbc:postgresql://localhost/postgres", p);
			Statement st = conn.createStatement();
			ResultSet rs = st.executeQuery("SELECT 1E+90");
		) {
			rs.next();
			System.out.println(rs.getBigDecimal(1));
			System.out.println(new BigDecimal(10).pow(90));
		}
	}
}

Expected behaviour
Expected 1 + 90 0 s but actually got 1 + 89 0 s.
SELECT 1E+89 got the expected value.

[davecramer]

Thx for the report. Seems PostgreSQL sends 100 instead of 10. Will look at it

[bokken]

Why are you wanting to enable binary support for numeric types? The binary format does not align well to the BigDecimal type, making for a fairly inefficient translation. The default string format ends up being less work.

[vlsi]

Even though binary vs text might be the same for numerics, we might want to support binary for numerics so arrays and structs can be sent in binary formats.

[bokken]

@vlsi it is not that binary vs text is /same/, it is that binary (particularly the current translation) appears highly likely to be slower.
Even then, this is only talking about the ability to receive numeric as binary, not send.

I have an implementation[1] of binary support of numeric that I worked on back in March. While this supports both sending and receiving and appears to be more efficient than the current implementation, I never created a PR because I could not find a reason to ever use binary over String (even for arrays).

[1] - master...bokken:numeric_binary

[vlsi]

it is that binary (particularly the current translation) appears highly likely to be slower.

I agree the current implementation does not look the most performant one, however, I believe binary mode unlocks binary for structs and arrays which might be way more efficient than coding/decoding arrays/structs into the text format.

this is only talking about the ability to receive numeric as binary

Technically speaking, it is either database sends shorts as is and we convert them to string or the database converts ints to a string and we receive the string.
The amount of works seems to be the same, however, it would probably be slightly better to ease the workload on the database, because scaling the applications is easier than scaling the database, so moving the conversion to the application seems to be a reasonable thing to do.

[vlsi]

even for arrays

That is strange. Do you have benchmarks by chance?

[bokken]

either database sends shorts as is

The binary format for these seems so odd to me (mix of decimal and binary) that I assumed some other form was actually used natively in the database that would be better for calculations (indexing, equality, comparisons, addition, etc.). This lead me to conclude that if the db is doing work to translate from internal form to either this binary format or string, and the java side handles the string much better, then just let the db go straight to string. If this binary format is the actual native format of the database, then I agree, it makes sense to do the conversion (even if ugly) in java.

even for arrays

That is strange. Do you have benchmarks by chance?

I do not. My observation from the code was that for single dimensional arrays, the handling of the numeric->big decimal would overwhelm the benefits of knowing the array length ahead of time.
Also, simply enabling this binary numeric support does not automatically enable binary support for numeric arrays. That requires enabling the array oid and adding code in ArrayDecoding.
As I discovered when working on improving binary support for arrays(#1194 (comment)), there was briefly binary support for numeric and numeric_array that had some problems (specifically around NaN), but those changes were partially lost during a merge.

This all feels a bit like deja-vu: #1749

[vlsi]

If this binary format is the actual native format of the database, then I agree, it makes sense to do the conversion (even if ugly) in java.

Currently, PostgreSQL calls binary format the one that is used internally by the database.

Here's numeric_send which is where the database sends the numeric in a binary form: https://github.com/postgres/postgres/blob/b4d5b458e6fb4c861f92888753deea6a5005a68d/src/backend/utils/adt/numeric.c#L1030

The text output is here: https://github.com/postgres/postgres/blob/b4d5b458e6fb4c861f92888753deea6a5005a68d/src/backend/utils/adt/numeric.c#L739