Confusing behavior of create table like
Postgres provides serial and bigserial column types for which it
implicitly creates sequence.
As far as this mechanism is somehow hidden from user, it may be
confusing that table
created with CREATE TABLE LIKE has no associated sequence.
But what is worse, even if experienced user knows that serial types are
implemented in Postgres by specifying
nextval(seq) default value for this column and default values are copied
by CREATE TABLE LIKE only if is it explicitly requested (including all),
then two tables will share the same sequence:
create table t1(x serial primary key, val int);
create table t2(like t1 including all);
postgres=# \d+ t1;
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage
| Stats target | Description
--------+---------+-----------+----------+-------------------------------+---------+--------------+-------------
x | integer | | not null |
nextval('t1_x_seq'::regclass) | plain | |
val | integer | | | |
plain | |
Indexes:
"t1_pkey" PRIMARY KEY, btree (x)
Access method: heap
postgres=# \d+ t2;
Table "public.t2"
Column | Type | Collation | Nullable | Default | Storage
| Stats target | Description
--------+---------+-----------+----------+-------------------------------+---------+--------------+-------------
x | integer | | not null |
nextval('t1_x_seq'::regclass) | plain | |
val | integer | | | |
plain | |
Indexes:
"t2_pkey" PRIMARY KEY, btree (x)
Access method: heap
Please notice that index is correctly replaced, but sequence - not.
I consider such behavior more like bug than a feature.
And it can be fixed using relatively small patch.
Thoughts?
Attachments:
create_table_like.patchtext/x-patch; charset=UTF-8; name=create_table_like.patchDownload
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 27b596cb59..cb56f9f246 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -831,6 +831,19 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
{
CookedConstraint *cooked;
+ /*
+ * If column has identity sequence but is not marked as identity column,
+ * then it means that this sequence was created by CREATE TABLE LIKE (see comment in
+ * transformTableLikeClause). We have to patch default expression, storing new sequence OID.
+ */
+ if (!colDef->identity && colDef->identitySequence)
+ {
+ FuncExpr* nextval = ((FuncExpr*)colDef->cooked_default)->funcid == F_NEXTVAL_OID
+ ? (FuncExpr*)colDef->cooked_default
+ : (FuncExpr*)linitial(((FuncExpr*)colDef->cooked_default)->args);
+ ((Const*)linitial(nextval->args))->constvalue = RangeVarGetRelid(colDef->identitySequence, NoLock, false);
+ colDef->identitySequence = NULL;
+ }
cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint));
cooked->contype = CONSTR_DEFAULT;
cooked->conoid = InvalidOid; /* until created */
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 25abc544fc..70296d02a2 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -64,6 +64,7 @@
#include "rewrite/rewriteManip.h"
#include "utils/acl.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/partcache.h"
#include "utils/rel.h"
@@ -913,6 +914,19 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
}
}
+/*
+ * Check if sequence was implicitly created by Postgres for SERIAL column
+ */
+static bool
+is_autogenerated_sequence(Relation rel, char* colname, Oid seqid)
+{
+ char* seqname = makeObjectName(RelationGetRelationName(rel),
+ colname,
+ "seq");
+ return strcmp(get_rel_name(seqid), seqname) == 0;
+}
+
+
/*
* transformTableLikeClause
*
@@ -1072,7 +1086,32 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
{
if (attrdef[i].adnum == parent_attno)
{
+ FuncExpr* nextval;
this_default = stringToNode(attrdef[i].adbin);
+ /*
+ * If sequence is implicitly generated for SERIAL or BIGSERIAL field,
+ * then default expression has for nextval('REL_COL_seq') or nextval('REL_COL_seq')::int
+ * Let's create new sequence for the target relation in this case.
+ * Since default expression is referencing sequence OID and we do not have it
+ * at this moment, then let DefineRelation do the work of patching default expression.
+ */
+ if (def->identitySequence == NULL
+ && IsA(this_default, FuncExpr)
+ && ((nextval = (FuncExpr*)this_default)->funcid == F_NEXTVAL_OID
+ || (((FuncExpr*)this_default)->funcid == F_INT84
+ && IsA(linitial(((FuncExpr*)this_default)->args), FuncExpr)
+ && (nextval = (FuncExpr*)linitial(((FuncExpr*)this_default)->args))->funcid == F_NEXTVAL_OID)))
+ {
+ Oid seq_relid = ((Const*)linitial(nextval->args))->constvalue;
+ List *seq_options = sequence_options(seq_relid);
+ if (is_autogenerated_sequence(relation, def->colname, seq_relid))
+ {
+ generateSerialExtraStmts(cxt, def,
+ InvalidOid, seq_options,
+ false, false,
+ NULL, NULL);
+ }
+ }
break;
}
}
On 2020-08-01 00:06, Konstantin Knizhnik wrote:
Postgres provides serial and bigserial column types for which it
implicitly creates sequence.
As far as this mechanism is somehow hidden from user, it may be
confusing that table
created with CREATE TABLE LIKE has no associated sequence.
That's why identity columns were added. You shouldn't use serial
columns anymore, especially if you are concerned about behaviors like this.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 03.08.2020 11:00, Peter Eisentraut wrote:
On 2020-08-01 00:06, Konstantin Knizhnik wrote:
Postgres provides serial and bigserial column types for which it
implicitly creates sequence.
As far as this mechanism is somehow hidden from user, it may be
confusing that table
created with CREATE TABLE LIKE has no associated sequence.That's why identity columns were added. You shouldn't use serial
columns anymore, especially if you are concerned about behaviors like
this.
I can completely agree with this position.
There are several things in Postgres which are conceptually similar,
share a lot of code but... following different rules.
Usually it happens when some new notion is introduced, fully or partly
substitute old notion.
Inheritance and declarative partitioning is one of such examples.
Although them are used to solve the same goal, there are many cases when
some optimization works for partitioned table but not for inheritance.
May be generated and identity columns are good things. I have nothing
against them.
But what preventing us from providing the similar behavior for
serial/bigseries types?
On Mon, Aug 3, 2020 at 8:59 AM Konstantin Knizhnik
<k.knizhnik@postgrespro.ru> wrote:
May be generated and identity columns are good things. I have nothing
against them.
But what preventing us from providing the similar behavior for
serial/bigseries types?
Backward compatibility seems like one good argument.
It kind of sucks that we end up with cases where new notions are
introduced to patch up the inadequacies of earlier ideas, but it's
also inevitable. If, after 25+ years of development, we didn't have
cases where somebody had come up with a new plan that was better than
the older plan, that would be pretty scary. We have to remember,
though, that there's a giant user community around PostgreSQL at this
point, and changing things like this can inconvenience large numbers
of those users. Sometimes that's worth it, but I find it pretty
dubious in a case like this. There's every possibility that there are
people out there who rely on the current behavior, and whose stuff
would break if it were changed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 2020-08-03 14:58, Konstantin Knizhnik wrote:
May be generated and identity columns are good things. I have nothing
against them.
But what preventing us from providing the similar behavior for
serial/bigseries types?
In my mind, serial/bigserial is deprecated and it's not worth spending
effort on patching them up.
One thing we could do is change serial/bigserial to expand to identity
column definitions instead of the current behavior.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Aug 3, 2020 at 12:35 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2020-08-03 14:58, Konstantin Knizhnik wrote:
May be generated and identity columns are good things. I have nothing
against them.
But what preventing us from providing the similar behavior for
serial/bigseries types?In my mind, serial/bigserial is deprecated and it's not worth spending
effort on patching them up.One thing we could do is change serial/bigserial to expand to identity
column definitions instead of the current behavior.
I'm not really convinced that's a good idea. There's probably a lot of
people (me included) who are used to the way serial and bigserial work
and wouldn't necessarily be happy about a change. Plus, aren't the
generated columns still depending on an underlying sequence anyway?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 04.08.2020 19:53, Robert Haas wrote:
On Mon, Aug 3, 2020 at 12:35 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 2020-08-03 14:58, Konstantin Knizhnik wrote:
May be generated and identity columns are good things. I have nothing
against them.
But what preventing us from providing the similar behavior for
serial/bigseries types?In my mind, serial/bigserial is deprecated and it's not worth spending
effort on patching them up.One thing we could do is change serial/bigserial to expand to identity
column definitions instead of the current behavior.I'm not really convinced that's a good idea. There's probably a lot of
people (me included) who are used to the way serial and bigserial work
and wouldn't necessarily be happy about a change. Plus, aren't the
generated columns still depending on an underlying sequence anyway?
Yes, generated columns are also using implicitly generated sequences.
So them are very similar with SERIAL/BIGSERIAL columns. This actually
make we wonder why we can not handle them in the same way in
CREATE TABLE LIKE.
The only difference is that it is not possible to explicitly specify
sequence for generated column.
And it certainly makes there handling in CREATE TABLE LIKE less
contradictory.
I think that many people are using serial/bigserial types in their
database schemas and will continue to use them.
I do not expect that any of them will be upset of behavior of handling
this columns in CREATE TABLE LIKE ... INCLUDING ALL will be changed.
Mostly because very few people are using this construction. But if
someone wants to use it, then most likely he will be confused
(I have not imagine this problem myself - it was motivated by question
in one of Postgres forums where current behavior was interpreted as bug).
So I do not think that "backward compatibility" is actually good in this
case and that somebody can suffer from changing it.
I do not insist - as I already told, I do not think that much people are
using CREATE TABLE LIKE, so it should not be a big problem.
But if there is some will to change current behavior, then I can send
more correct version of the patch and may be submit it to commitfest.
On 2020-08-04 19:36, Konstantin Knizhnik wrote:
Yes, generated columns are also using implicitly generated sequences.
So them are very similar with SERIAL/BIGSERIAL columns. This actually
make we wonder why we can not handle them in the same way in
CREATE TABLE LIKE.
The current specification of serial is a parse-time expansion of integer
column, sequence, and column default. The behavior of column defaults
in CREATE TABLE LIKE does not currently include rewriting the default
expression or creating additional schema objects. If you want to
introduce these concepts, it should be done in a general way, not just
hard-coded for a particular case.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 2020-08-04 19:36, Konstantin Knizhnik wrote:
Yes, generated columns are also using implicitly generated sequences.
So them are very similar with SERIAL/BIGSERIAL columns. This actually
make we wonder why we can not handle them in the same way in
CREATE TABLE LIKE.
The current specification of serial is a parse-time expansion of integer
column, sequence, and column default.
Yeah; and note it's actually defined that way in the docs.
I'd certainly concede that serial is a legacy feature now that we have
identity columns. But, by the same token, its value is in backwards
compatibility with old behaviors. Therefore, reimplementing it in a
way that isn't 100% backwards compatible seems like entirely the
wrong thing to do. On similar grounds, I'd be pretty suspicious of
changing LIKE's behaviors around the case.
regards, tom lane