Thread: BUG #15096: Unable to CREATE TABLE LIKE with bigint identity column

BUG #15096: Unable to CREATE TABLE LIKE with bigint identity column

From
PG Bug reporting form
Date:
The following bug has been logged on the website:

Bug reference:      15096
Logged by:          Evgeny
Email address:      evgeni-klimov@yandex.ru
PostgreSQL version: 10.2
Operating system:   Windows 7 x64
Description:

CREATE TABLE public.test (id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY
KEY, s TEXT);
CREATE TABLE public.test_copy (LIKE public.test INCLUDING ALL);
ERROR:  MINVALUE (1) must be less than MAXVALUE (-1)

However, if I change the id column type to INT, the second command
succeeds.
Checked on 10.3 and 10.2.


=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> CREATE TABLE public.test (id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY
> KEY, s TEXT);
> CREATE TABLE public.test_copy (LIKE public.test INCLUDING ALL);
> ERROR:  MINVALUE (1) must be less than MAXVALUE (-1)

Hm ... works fine for me on a 64-bit Linux machine ... but it fails
as described on 32-bit.  Something in the LIKE code path is shoving
the sequence's maxvalue through a variable with platform-dependent
width, probably of type "long".  No time right now to locate the
exact problem.

            regards, tom lane


Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identity column

From
Andrew Gierth
Date:
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 >> ERROR:  MINVALUE (1) must be less than MAXVALUE (-1)

 Tom> Hm ... works fine for me on a 64-bit Linux machine ... but it
 Tom> fails as described on 32-bit. Something in the LIKE code path is
 Tom> shoving the sequence's maxvalue through a variable with
 Tom> platform-dependent width, probably of type "long". No time right
 Tom> now to locate the exact problem.

The Value node's "ival" field is declared as a long.

sequence_options thinks it can put all the sequence parameters through
makeInteger and have them come out intact.

-- 
Andrew (irc:RhodiumToad)


Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Michael Paquier
Date:
On Sat, Mar 03, 2018 at 07:55:41PM +0000, Andrew Gierth wrote:
> sequence_options thinks it can put all the sequence parameters through
> makeInteger and have them come out intact.

Yeah, on 32b machines "long" is 4 bytes...  Perhaps it would be the
occasion to introduce a T_Integer64 type for Value which gets stored as
a string?  And as far as I can see defGetInt64 is only used by
sequences.
--
Michael

Attachment

Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identity column

From
Andrew Gierth
Date:
>>>>> "Michael" == Michael Paquier <michael@paquier.xyz> writes:

 > On Sat, Mar 03, 2018 at 07:55:41PM +0000, Andrew Gierth wrote:
 >> sequence_options thinks it can put all the sequence parameters
 >> through makeInteger and have them come out intact.

 Michael> Yeah, on 32b machines "long" is 4 bytes...

And on 64-bit Windows, too.

 Michael> Perhaps it would be the occasion to introduce a T_Integer64
 Michael> type for Value which gets stored as a string? And as far as I
 Michael> can see defGetInt64 is only used by sequences.

The slightly misnamed T_Float is what's currently used for Value nodes
which contain numeric values as strings. So there'd be no point in a new
type tag if you're still going to store the value as a string.

-- 
Andrew (irc:RhodiumToad)


Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Michael" == Michael Paquier <michael@paquier.xyz> writes:
>  Michael> Perhaps it would be the occasion to introduce a T_Integer64
>  Michael> type for Value which gets stored as a string? And as far as I
>  Michael> can see defGetInt64 is only used by sequences.

> The slightly misnamed T_Float is what's currently used for Value nodes
> which contain numeric values as strings. So there'd be no point in a new
> type tag if you're still going to store the value as a string.

Going forward, maybe we should change the T_Integer case to either int64
or int32, so that it's not got a platform-dependent range.  That's not a
workable solution for back-patching into v10, though (and neither is
T_Integer64, really).

            regards, tom lane


Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Michael Paquier
Date:
On Mon, Mar 05, 2018 at 12:44:32AM -0500, Tom Lane wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>> "Michael" == Michael Paquier <michael@paquier.xyz> writes:
>>  Michael> Perhaps it would be the occasion to introduce a T_Integer64
>>  Michael> type for Value which gets stored as a string? And as far as I
>>  Michael> can see defGetInt64 is only used by sequences.
>
>> The slightly misnamed T_Float is what's currently used for Value nodes
>> which contain numeric values as strings. So there'd be no point in a new
>> type tag if you're still going to store the value as a string.
>
> Going forward, maybe we should change the T_Integer case to either int64
> or int32, so that it's not got a platform-dependent range.

Serial columns using bigint as type would need int64 anyway, no?  Why
int32?

> That's not a workable solution for back-patching into v10, though (and
> neither is T_Integer64, really).

Sure.  For v10, using just T_Float should be doable at quick glance.  I
have not checked though.
--
Michael

Attachment
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Mar 05, 2018 at 12:44:32AM -0500, Tom Lane wrote:
>> Going forward, maybe we should change the T_Integer case to either int64
>> or int32, so that it's not got a platform-dependent range.

> Serial columns using bigint as type would need int64 anyway, no?  Why
> int32?

int32 might be less work to make fully portable.  I don't recall exactly
what-all we do with T_Integer, but if we try to do sscanf() to produce the
value for instance, that's a problem.  (Note that configure's tests for
64-bit support only cover sprintf, not sscanf.)

Certainly int64 would be a more forward-looking choice, it just seems
like possibly more work.

            regards, tom lane


Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Peter Eisentraut
Date:
Here is a patch that fixes this bug, and a second patch (not for
backpatching) that changes the Value node to use int instead of long.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Michael Paquier
Date:
On Wed, Mar 07, 2018 at 03:16:44PM -0500, Peter Eisentraut wrote:
> Here is a patch that fixes this bug, and a second patch (not for
> backpatching) that changes the Value node to use int instead of long.

Thanks Peter for diving in.  Patch 1 looks fine to me.

Here are some comments for patch 2.

+       if (endptr != token + length || errno == ERANGE ||
+           /* check for overflow of int4 */
+           val != (long) ((int32) val))
            return T_Float;
It would be nice to have this check be consistent with the new
definition of ival and int32, One suggestion is to use directly int32 or
just have a static assertion that sizeof(int) == sizeof(int32)?  Or
that's too much nannyism?

By the way, why do you remove HAVE_LONG_INT_64?  On platforms where long
is 4 bytes this would still be a no-op.  If you care about those, you
could also remove the ones in interval.c and datetime.c...

The comment block on top of the definition of Value in value.h still
mentions "long", while it should mention "int" per your patch.
--
Michael

Attachment

Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Peter Eisentraut
Date:
On 3/8/18 03:08, Michael Paquier wrote:
> Here are some comments for patch 2.
> 
> +       if (endptr != token + length || errno == ERANGE ||
> +           /* check for overflow of int4 */
> +           val != (long) ((int32) val))
>             return T_Float;
> It would be nice to have this check be consistent with the new
> definition of ival and int32, One suggestion is to use directly int32 or
> just have a static assertion that sizeof(int) == sizeof(int32)?  Or
> that's too much nannyism?

Fixed in the attached next version.

> By the way, why do you remove HAVE_LONG_INT_64?  On platforms where long
> is 4 bytes this would still be a no-op.

Right, but the compiler can optimize it away then.  No need to have #ifdefs.

> The comment block on top of the definition of Value in value.h still
> mentions "long", while it should mention "int" per your patch.  

done

> If you care about those, you
> could also remove the ones in interval.c and datetime.c...

Actually, we could just use the strtoint() defined there and apply it
everywhere, so avoid repeating these patterns.  Done so in an additional
patch.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Michael Paquier
Date:
On Mon, Mar 12, 2018 at 01:45:26PM -0400, Peter Eisentraut wrote:
> On 3/8/18 03:08, Michael Paquier wrote:
>> Here are some comments for patch 2.
>>
>> +       if (endptr != token + length || errno == ERANGE ||
>> +           /* check for overflow of int4 */
>> +           val != (long) ((int32) val))
>>             return T_Float;
>> It would be nice to have this check be consistent with the new
>> definition of ival and int32, One suggestion is to use directly int32 or
>> just have a static assertion that sizeof(int) == sizeof(int32)?  Or
>> that's too much nannyism?
>
> Fixed in the attached next version.

Thanks, this looks good to me.

>> If you care about those, you
>> could also remove the ones in interval.c and datetime.c...
>
> Actually, we could just use the strtoint() defined there and apply it
> everywhere, so avoid repeating these patterns.  Done so in an additional
> patch.

OK, that's a good idea.

+/*
+ * strtoint --- just like strtol, but returns int not long
+ */
It would be nice to document that caller should check for errno for
sanity checks, in which case caller should not rely on the returned
result.

Worth noting that pgbench has its own view of things with strtoint64.
Not worth bothering anyway.
--
Michael

Attachment

Re: BUG #15096: Unable to CREATE TABLE LIKE with bigint identitycolumn

From
Peter Eisentraut
Date:
On 3/13/18 02:22, Michael Paquier wrote:
> Thanks, this looks good to me.

committed (0001 to PG10, 0001-0003 to master)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services