fix(c/driver/postgresql): fix numeric to str

[lupko]

• conversion from numeric to string had two bugs due to deviations from the original PostgreSQL code
• leading single zero before decimal would always be dropped
• in some cases, the numbers after decimal would not be incomplete and instead replaced with 'garbage'

here is the PostgreSQL code: https://github.com/postgres/postgres/blob/9589b038d3203cd5ba708fb4f5c23182c88ad0b3/src/backend/utils/adt/numeric.c#L7443

(the DEC_DIGITS=4 variant)

Fixes #1501.

[lidavidm]

Thanks! Is it possible to add a test case for this?

There's an example here:

arrow-adbc/c/driver/postgresql/copy/postgres_copy_reader_test.cc

Line 317 in d6694a2

TEST(PostgresCopyUtilsTest, PostgresCopyReadNumeric) {

Though, I'm not sure how the reference data was generated; it looks to be by running a query and saving the output/translating it to a byte array:

arrow-adbc/c/driver/postgresql/copy/postgres_copy_test_common.h

Lines 155 to 176 in d6694a2

	// For full coverage, ensure that this contains NUMERIC examples that:
	// - Have >= four zeroes to the left of the decimal point
	// - Have >= four zeroes to the right of the decimal point
	// - Include special values (nan, -inf, inf, NULL)
	// - Have >= four trailing zeroes to the right of the decimal point
	// - Have >= four leading zeroes before the first digit to the right of the decimal point
	// - Is < 0 (negative)
	// COPY (SELECT CAST(col AS NUMERIC) AS col FROM ( VALUES (1000000), ('0.00001234'),
	// ('1.0000'), (-123.456), (123.456), ('nan'), ('-inf'), ('inf'), (NULL)) AS drvd(col)) TO
	// STDOUT WITH (FORMAT binary);
	static const uint8_t kTestPgCopyNumeric[] = {
	0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, 0x00, 0x00,
	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x01, 0x00,
	0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0a, 0x00,
	0x01, 0xff, 0xfe, 0x00, 0x00, 0x00, 0x08, 0x04, 0xd2, 0x00, 0x01, 0x00, 0x00, 0x00,
	0x0a, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x01, 0x00, 0x01, 0x00,
	0x00, 0x00, 0x0c, 0x00, 0x02, 0x00, 0x00, 0x40, 0x00, 0x00, 0x03, 0x00, 0x7b, 0x11,
	0xd0, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
	0x03, 0x00, 0x7b, 0x11, 0xd0, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
	0x00, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
	0x00, 0xf0, 0x00, 0x00, 0x20, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
	0x00, 0xd0, 0x00, 0x00, 0x20, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};

[WillAyd]

Though, I'm not sure how the reference data was generated; it looks to be by running a query and saving the output/translating it to a byte array:

What I typically do is COPY out to a file then check the file in a hex editor. That way nothing with the terminal or keyring ends mixing up the bytes e.g.

postgres=# COPY (SELECT CAST(col AS NUMERIC) AS col FROM (  VALUES (1000000), ('0.00001234'), ('1.0000'), (-123.456), (123.456), ('nan'), ('-inf'), ('inf'), (NULL)) AS drvd(col)) TO '/tmp/pgdata.out' WITH (FORMAT binary);

Will write the bytes out to /tmp/pgdata.out

[lupko]

Added extra test against NUMERIC(16,10). Tried with the fix reverted - test was failing. Both the missing single zero before decimal was missing & there was corruption on the tail end.

the test data does not contain +- infinity - as per PostgreSQL doc, these can be only stored in unconstrained NUMERIC.

[lidavidm]

LGTM, thanks. I'll merge after CI unless Will has comments

[lupko]

eih, sorry CI has failed because I did not have env setup with pre-commit so some code failed the format check. corrected.

[WillAyd]

lgtm - thanks for the investigation and fix!