nodes/*funcs.c inconsistencies
I observed these inconsistencies in node support functions:
- _copyReassignOwnedStmt() uses COPY_SCALAR_FIELD() on the string field
"newrole", and _equalReassignOwnedStmt() uses COMPARE_NODE_FIELD().
- _outCreateForeignTableStmt() calls _outCreateStmt() directly. This produces
the label "CREATEFOREIGNTABLESTMTCREATESTMT". The attached patch splits
things out the way we normally do in outfuncs.c. There's no readfuncs.c
support, so this is strictly cosmetic.
- _outColumnDef() uses WRITE_INT_FIELD for the "storage" field, a char.
Again, no readfuncs.c support to create a compatibility problem.
- _copyRenameStmt() and _equalRenameStmt() ignore the "relationType" field,
but I can't see a good reason to do so. PostgreSQL 9.1 added this field,
but only recent master (after commit 38b9693f of 3 April 2012) references
the field beyond setting it in the parser.
- _copyViewStmt() and _equalViewStmt() ignore the "options" field, and
_equalColumnDef() ignores "fdwoptions". These are new in PostgreSQL 9.2,
and I see no good reason to ignore them.
I'd suggest backpatching the ReassignOwnedStmt() bits; the wrong code could
produce crashes. The rest are for master only.
Thanks,
nm
Attachments:
node-support-cleanup-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index c94799b..eed0ea4 100644
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 2891,2896 **** _copyRenameStmt(const RenameStmt *from)
--- 2891,2897 ----
RenameStmt *newnode = makeNode(RenameStmt);
COPY_SCALAR_FIELD(renameType);
+ COPY_SCALAR_FIELD(relationType);
COPY_NODE_FIELD(relation);
COPY_NODE_FIELD(object);
COPY_NODE_FIELD(objarg);
***************
*** 3047,3052 **** _copyViewStmt(const ViewStmt *from)
--- 3048,3054 ----
COPY_NODE_FIELD(aliases);
COPY_NODE_FIELD(query);
COPY_SCALAR_FIELD(replace);
+ COPY_NODE_FIELD(options);
return newnode;
}
***************
*** 3650,3656 **** _copyReassignOwnedStmt(const ReassignOwnedStmt *from)
ReassignOwnedStmt *newnode = makeNode(ReassignOwnedStmt);
COPY_NODE_FIELD(roles);
! COPY_SCALAR_FIELD(newrole);
return newnode;
}
--- 3652,3658 ----
ReassignOwnedStmt *newnode = makeNode(ReassignOwnedStmt);
COPY_NODE_FIELD(roles);
! COPY_STRING_FIELD(newrole);
return newnode;
}
diff --git a/src/backend/nodes/equalindex 9564210..c06b068 100644
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***************
*** 1306,1311 **** static bool
--- 1306,1312 ----
_equalRenameStmt(const RenameStmt *a, const RenameStmt *b)
{
COMPARE_SCALAR_FIELD(renameType);
+ COMPARE_SCALAR_FIELD(relationType);
COMPARE_NODE_FIELD(relation);
COMPARE_NODE_FIELD(object);
COMPARE_NODE_FIELD(objarg);
***************
*** 1438,1443 **** _equalViewStmt(const ViewStmt *a, const ViewStmt *b)
--- 1439,1445 ----
COMPARE_NODE_FIELD(aliases);
COMPARE_NODE_FIELD(query);
COMPARE_SCALAR_FIELD(replace);
+ COMPARE_NODE_FIELD(options);
return true;
}
***************
*** 1945,1951 **** static bool
_equalReassignOwnedStmt(const ReassignOwnedStmt *a, const ReassignOwnedStmt *b)
{
COMPARE_NODE_FIELD(roles);
! COMPARE_NODE_FIELD(newrole);
return true;
}
--- 1947,1953 ----
_equalReassignOwnedStmt(const ReassignOwnedStmt *a, const ReassignOwnedStmt *b)
{
COMPARE_NODE_FIELD(roles);
! COMPARE_STRING_FIELD(newrole);
return true;
}
***************
*** 2182,2187 **** _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
--- 2184,2190 ----
COMPARE_NODE_FIELD(collClause);
COMPARE_SCALAR_FIELD(collOid);
COMPARE_NODE_FIELD(constraints);
+ COMPARE_NODE_FIELD(fdwoptions);
return true;
}
diff --git a/src/backend/nodes/outfunindex 594b3fd..f713773 100644
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 1927,1937 **** _outPlannerParamItem(StringInfo str, const PlannerParamItem *node)
*
*****************************************************************************/
static void
! _outCreateStmt(StringInfo str, const CreateStmt *node)
{
- WRITE_NODE_TYPE("CREATESTMT");
-
WRITE_NODE_FIELD(relation);
WRITE_NODE_FIELD(tableElts);
WRITE_NODE_FIELD(inhRelations);
--- 1927,1938 ----
*
*****************************************************************************/
+ /*
+ * print the basic stuff of all nodes that inherit from CreateStmt
+ */
static void
! _outCreateStmtInfo(StringInfo str, const CreateStmt *node)
{
WRITE_NODE_FIELD(relation);
WRITE_NODE_FIELD(tableElts);
WRITE_NODE_FIELD(inhRelations);
***************
*** 1944,1954 **** _outCreateStmt(StringInfo str, const CreateStmt *node)
}
static void
_outCreateForeignTableStmt(StringInfo str, const CreateForeignTableStmt *node)
{
WRITE_NODE_TYPE("CREATEFOREIGNTABLESTMT");
! _outCreateStmt(str, (const CreateStmt *) &node->base);
WRITE_STRING_FIELD(servername);
WRITE_NODE_FIELD(options);
--- 1945,1963 ----
}
static void
+ _outCreateStmt(StringInfo str, const CreateStmt *node)
+ {
+ WRITE_NODE_TYPE("CREATESTMT");
+
+ _outCreateStmtInfo(str, (const CreateStmt *) node);
+ }
+
+ static void
_outCreateForeignTableStmt(StringInfo str, const CreateForeignTableStmt *node)
{
WRITE_NODE_TYPE("CREATEFOREIGNTABLESTMT");
! _outCreateStmtInfo(str, (const CreateStmt *) node);
WRITE_STRING_FIELD(servername);
WRITE_NODE_FIELD(options);
***************
*** 2088,2094 **** _outColumnDef(StringInfo str, const ColumnDef *node)
WRITE_BOOL_FIELD(is_local);
WRITE_BOOL_FIELD(is_not_null);
WRITE_BOOL_FIELD(is_from_type);
! WRITE_INT_FIELD(storage);
WRITE_NODE_FIELD(raw_default);
WRITE_NODE_FIELD(cooked_default);
WRITE_NODE_FIELD(collClause);
--- 2097,2103 ----
WRITE_BOOL_FIELD(is_local);
WRITE_BOOL_FIELD(is_not_null);
WRITE_BOOL_FIELD(is_from_type);
! WRITE_CHAR_FIELD(storage);
WRITE_NODE_FIELD(raw_default);
WRITE_NODE_FIELD(cooked_default);
WRITE_NODE_FIELD(collClause);
On Mon, Apr 16, 2012 at 6:25 AM, Noah Misch <noah@leadboat.com> wrote:
I'd suggest backpatching the ReassignOwnedStmt() bits; the wrong code could
produce crashes. The rest are for master only.
Done, in the manner you suggest.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Excerpts from Robert Haas's message of mié abr 18 11:47:37 -0300 2012:
On Mon, Apr 16, 2012 at 6:25 AM, Noah Misch <noah@leadboat.com> wrote:
I'd suggest backpatching the ReassignOwnedStmt() bits; the wrong code could
produce crashes. The rest are for master only.Done, in the manner you suggest.
Pah. I was just done with it and was about to push it ...
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, Apr 16, 2012 at 06:25:15AM -0400, Noah Misch wrote:
I observed these inconsistencies in node support functions:
A fresh audit found the attached problems new in 9.5[1]The _equalCreateEventTrigStmt() cosmetic change is relevant as far back as 9.3, but I won't back-patch it further than the others (9.5).. Most are cosmetic
INT/UINT or field order corrections. The non-cosmetic changes involve
CustomPath, CustomScan, and CreatePolicyStmt. Feature committers, if the
existing treatments (ignore custom_plans/custom_paths fields; copy/compare
"cmd" string pointer as a scalar) were deliberate, please let me know.
Thanks,
nm
[1]: The _equalCreateEventTrigStmt() cosmetic change is relevant as far back as 9.3, but I won't back-patch it further than the others (9.5).
9.3, but I won't back-patch it further than the others (9.5).
Attachments:
nodefuncs-95features-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 7248440..1c8425d 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -647,6 +647,7 @@ _copyCustomScan(const CustomScan *from)
* copy remainder of node
*/
COPY_SCALAR_FIELD(flags);
+ COPY_NODE_FIELD(custom_plans);
COPY_NODE_FIELD(custom_exprs);
COPY_NODE_FIELD(custom_private);
COPY_NODE_FIELD(custom_scan_tlist);
@@ -1925,9 +1926,9 @@ _copyOnConflictExpr(const OnConflictExpr *from)
COPY_SCALAR_FIELD(action);
COPY_NODE_FIELD(arbiterElems);
COPY_NODE_FIELD(arbiterWhere);
+ COPY_SCALAR_FIELD(constraint);
COPY_NODE_FIELD(onConflictSet);
COPY_NODE_FIELD(onConflictWhere);
- COPY_SCALAR_FIELD(constraint);
COPY_SCALAR_FIELD(exclRelIndex);
COPY_NODE_FIELD(exclRelTlist);
@@ -4082,7 +4083,7 @@ _copyCreatePolicyStmt(const CreatePolicyStmt *from)
COPY_STRING_FIELD(policy_name);
COPY_NODE_FIELD(table);
- COPY_SCALAR_FIELD(cmd);
+ COPY_STRING_FIELD(cmd);
COPY_NODE_FIELD(roles);
COPY_NODE_FIELD(qual);
COPY_NODE_FIELD(with_check);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 6597dbc..1d6c43c 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -759,9 +759,9 @@ _equalOnConflictExpr(const OnConflictExpr *a, const OnConflictExpr *b)
COMPARE_SCALAR_FIELD(action);
COMPARE_NODE_FIELD(arbiterElems);
COMPARE_NODE_FIELD(arbiterWhere);
+ COMPARE_SCALAR_FIELD(constraint);
COMPARE_NODE_FIELD(onConflictSet);
COMPARE_NODE_FIELD(onConflictWhere);
- COMPARE_SCALAR_FIELD(constraint);
COMPARE_SCALAR_FIELD(exclRelIndex);
COMPARE_NODE_FIELD(exclRelTlist);
@@ -1868,8 +1868,8 @@ _equalCreateEventTrigStmt(const CreateEventTrigStmt *a, const CreateEventTrigStm
{
COMPARE_STRING_FIELD(trigname);
COMPARE_STRING_FIELD(eventname);
- COMPARE_NODE_FIELD(funcname);
COMPARE_NODE_FIELD(whenclause);
+ COMPARE_NODE_FIELD(funcname);
return true;
}
@@ -2074,7 +2074,7 @@ _equalCreatePolicyStmt(const CreatePolicyStmt *a, const CreatePolicyStmt *b)
{
COMPARE_STRING_FIELD(policy_name);
COMPARE_NODE_FIELD(table);
- COMPARE_SCALAR_FIELD(cmd);
+ COMPARE_STRING_FIELD(cmd);
COMPARE_NODE_FIELD(roles);
COMPARE_NODE_FIELD(qual);
COMPARE_NODE_FIELD(with_check);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 86f3214..068724e 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -341,7 +341,7 @@ _outModifyTable(StringInfo str, const ModifyTable *node)
WRITE_NODE_FIELD(arbiterIndexes);
WRITE_NODE_FIELD(onConflictSet);
WRITE_NODE_FIELD(onConflictWhere);
- WRITE_INT_FIELD(exclRelRTI);
+ WRITE_UINT_FIELD(exclRelRTI);
WRITE_NODE_FIELD(exclRelTlist);
}
@@ -591,6 +591,7 @@ _outCustomScan(StringInfo str, const CustomScan *node)
_outScanInfo(str, (const Scan *) node);
WRITE_UINT_FIELD(flags);
+ WRITE_NODE_FIELD(custom_plans);
WRITE_NODE_FIELD(custom_exprs);
WRITE_NODE_FIELD(custom_private);
WRITE_NODE_FIELD(custom_scan_tlist);
@@ -1016,7 +1017,7 @@ _outGroupingFunc(StringInfo str, const GroupingFunc *node)
WRITE_NODE_FIELD(args);
WRITE_NODE_FIELD(refs);
WRITE_NODE_FIELD(cols);
- WRITE_INT_FIELD(agglevelsup);
+ WRITE_UINT_FIELD(agglevelsup);
WRITE_LOCATION_FIELD(location);
}
@@ -1532,9 +1533,9 @@ _outOnConflictExpr(StringInfo str, const OnConflictExpr *node)
WRITE_ENUM_FIELD(action, OnConflictAction);
WRITE_NODE_FIELD(arbiterElems);
WRITE_NODE_FIELD(arbiterWhere);
+ WRITE_OID_FIELD(constraint);
WRITE_NODE_FIELD(onConflictSet);
WRITE_NODE_FIELD(onConflictWhere);
- WRITE_OID_FIELD(constraint);
WRITE_INT_FIELD(exclRelIndex);
WRITE_NODE_FIELD(exclRelTlist);
}
@@ -1674,6 +1675,7 @@ _outCustomPath(StringInfo str, const CustomPath *node)
_outPathInfo(str, (const Path *) node);
WRITE_UINT_FIELD(flags);
+ WRITE_NODE_FIELD(custom_paths);
WRITE_NODE_FIELD(custom_private);
appendStringInfoString(str, " :methods ");
_outToken(str, node->methods->CustomName);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 71be840..23e0b36 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -538,7 +538,7 @@ _readGroupingFunc(void)
READ_NODE_FIELD(args);
READ_NODE_FIELD(refs);
READ_NODE_FIELD(cols);
- READ_INT_FIELD(agglevelsup);
+ READ_UINT_FIELD(agglevelsup);
READ_LOCATION_FIELD(location);
READ_DONE();
@@ -1256,9 +1256,9 @@ _readOnConflictExpr(void)
READ_ENUM_FIELD(action, OnConflictAction);
READ_NODE_FIELD(arbiterElems);
READ_NODE_FIELD(arbiterWhere);
+ READ_OID_FIELD(constraint);
READ_NODE_FIELD(onConflictSet);
READ_NODE_FIELD(onConflictWhere);
- READ_OID_FIELD(constraint);
READ_INT_FIELD(exclRelIndex);
READ_NODE_FIELD(exclRelTlist);
Noah Misch <noah@leadboat.com> writes:
On Mon, Apr 16, 2012 at 06:25:15AM -0400, Noah Misch wrote:
I observed these inconsistencies in node support functions:
A fresh audit found the attached problems new in 9.5[1].
Many thanks for doing that; I'd had the same checking on my personal to-do
list, but now will not need to.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Aug 2, 2015 at 1:28 PM, Noah Misch <noah@leadboat.com> wrote:
A fresh audit found the attached problems new in 9.5[1]. Most are cosmetic
INT/UINT or field order corrections.
I was responsible for a couple of the cosmetic ones. Sorry about that.
It occurs to me that we could do a little more to prevent this
automatically. Couldn't we adopt
AssertVariableIsOfType()/AssertVariableIsOfTypeMacro() to macros like
READ_UINT_FIELD()?
I'm surprised that this stuff was only ever used for logical decoding
infrastructure so far.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Aug 2, 2015 at 3:07 PM, Peter Geoghegan <pg@heroku.com> wrote:
I'm surprised that this stuff was only ever used for logical decoding
infrastructure so far.
On second thought, having tried it, one reason is that that breaks
things that are considered legitimate for historical reasons. For
example, AttrNumber is often used with READ_INT_FIELD(), which is an
int16. Whether or not it's worth fixing that by introducing a
READ_ATTRNUM_FIELD() (and so on) is not obvious to me.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Noah,
* Noah Misch (noah@leadboat.com) wrote:
On Mon, Apr 16, 2012 at 06:25:15AM -0400, Noah Misch wrote:
I observed these inconsistencies in node support functions:
A fresh audit found the attached problems new in 9.5[1]. Most are cosmetic
INT/UINT or field order corrections. The non-cosmetic changes involve
CustomPath, CustomScan, and CreatePolicyStmt. Feature committers, if the
existing treatments (ignore custom_plans/custom_paths fields; copy/compare
"cmd" string pointer as a scalar) were deliberate, please let me know.
Thanks for the review. The change you have is correct for
CreatePolicyStmt, at least. I imagine I confused it with polcmd, which
is actually just a char.
Barring objections, I'll change it to cmd_name after your commit, to
reduce the chances of future confusion.
Thanks again!
Stephen
On Mon, Apr 16, 2012 at 06:25:15AM -0400, Noah Misch wrote:
I observed these inconsistencies in node support functions:
A fresh audit found the attached problems new in 9.5[1]. Most are cosmetic
INT/UINT or field order corrections. The non-cosmetic changes involve
CustomPath, CustomScan, and CreatePolicyStmt. Feature committers, if the
existing treatments (ignore custom_plans/custom_paths fields; copy/compare
"cmd" string pointer as a scalar) were deliberate, please let me know.
Thanks for your works.
I also noticed one other inconsistent point; _outMergeJoin() dumps
mergeNullsFirst[] array but it does not use booltostr() macro.
Best regards,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Aug 03, 2015 at 01:32:10AM +0000, Kouhei Kaigai wrote:
On Mon, Apr 16, 2012 at 06:25:15AM -0400, Noah Misch wrote:
I observed these inconsistencies in node support functions:
A fresh audit found the attached problems new in 9.5[1]. Most are cosmetic
INT/UINT or field order corrections. The non-cosmetic changes involve
CustomPath, CustomScan, and CreatePolicyStmt. Feature committers, if the
existing treatments (ignore custom_plans/custom_paths fields; copy/compare
"cmd" string pointer as a scalar) were deliberate, please let me know.Thanks for your works.
I also noticed one other inconsistent point; _outMergeJoin() dumps
mergeNullsFirst[] array but it does not use booltostr() macro.
Good catch. All supported branches work that way, and it's not wrong. I
recommend keeping it unchanged.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Stephen Frost <sfrost@snowman.net> writes:
Noah,
A fresh audit found the attached problems new in 9.5[1]. Most are cosmetic
INT/UINT or field order corrections. The non-cosmetic changes involve
CustomPath, CustomScan, and CreatePolicyStmt. Feature committers, if the
existing treatments (ignore custom_plans/custom_paths fields; copy/compare
"cmd" string pointer as a scalar) were deliberate, please let me know.
Thanks for the review. The change you have is correct for
CreatePolicyStmt, at least. I imagine I confused it with polcmd, which
is actually just a char.
Barring objections, I'll change it to cmd_name after your commit, to
reduce the chances of future confusion.
Both of you please keep in mind that these "cosmetic" changes are
initdb-forcing, at least if they affect node types that can appear
in stored rules.
That being the case, it would probably be a good idea to get them done
before alpha2, as there may not be a good opportunity afterwards.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Aug 02, 2015 at 11:31:16PM -0400, Tom Lane wrote:
Stephen Frost <sfrost@snowman.net> writes:
Noah,
A fresh audit found the attached problems new in 9.5[1]. Most are cosmetic
INT/UINT or field order corrections. The non-cosmetic changes involve
CustomPath, CustomScan, and CreatePolicyStmt. Feature committers, if the
existing treatments (ignore custom_plans/custom_paths fields; copy/compare
"cmd" string pointer as a scalar) were deliberate, please let me know.Thanks for the review. The change you have is correct for
CreatePolicyStmt, at least. I imagine I confused it with polcmd, which
is actually just a char.Barring objections, I'll change it to cmd_name after your commit, to
reduce the chances of future confusion.
The existing identifier seems fine, but won't I mind that change, either.
Both of you please keep in mind that these "cosmetic" changes are
initdb-forcing, at least if they affect node types that can appear
in stored rules.
Right; Stephen's does not force initdb, but some of what I posted does so.
That being the case, it would probably be a good idea to get them done
before alpha2, as there may not be a good opportunity afterwards.
Freedom to bump catversion after alpha2 will be barely-distinguishable from
freedom to do so now. I have planned to leave my usual comment period of a
few days, though skipping that would be rather innocuous in this case.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Noah Misch (noah@leadboat.com) wrote:
On Sun, Aug 02, 2015 at 11:31:16PM -0400, Tom Lane wrote:
That being the case, it would probably be a good idea to get them done
before alpha2, as there may not be a good opportunity afterwards.Freedom to bump catversion after alpha2 will be barely-distinguishable from
freedom to do so now. I have planned to leave my usual comment period of a
few days, though skipping that would be rather innocuous in this case.
This is clearly necessary, of course, and I wouldn't be surprised if we
have another necessary bump post-alpha2, but at the same time, it'd
certainly be nice if we are able to put out alpha2 and then a beta1 in
the future without needing to do a bump.
In other words, +1 from me for going ahead and committing this for
alpha2, if possible.
Thanks!
Stephen
Peter Geoghegan wrote:
On Sun, Aug 2, 2015 at 3:07 PM, Peter Geoghegan <pg@heroku.com> wrote:
I'm surprised that this stuff was only ever used for logical decoding
infrastructure so far.On second thought, having tried it, one reason is that that breaks
things that are considered legitimate for historical reasons. For
example, AttrNumber is often used with READ_INT_FIELD(), which is an
int16. Whether or not it's worth fixing that by introducing a
READ_ATTRNUM_FIELD() (and so on) is not obvious to me.
If it allows us to introduce additional checking for new code, I'm all
for it.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/03/2015 09:55 AM, Stephen Frost wrote:
* Noah Misch (noah@leadboat.com) wrote:
On Sun, Aug 02, 2015 at 11:31:16PM -0400, Tom Lane wrote:
That being the case, it would probably be a good idea to get
them done before alpha2, as there may not be a good opportunity
afterwards.Freedom to bump catversion after alpha2 will be
barely-distinguishable from freedom to do so now. I have planned
to leave my usual comment period of a few days, though skipping
that would be rather innocuous in this case.This is clearly necessary, of course, and I wouldn't be surprised
if we have another necessary bump post-alpha2, but at the same
time, it'd certainly be nice if we are able to put out alpha2 and
then a beta1 in the future without needing to do a bump.In other words, +1 from me for going ahead and committing this for
alpha2, if possible.
+1
- --
Joe Conway
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJVv52QAAoJEDfy90M199hl/skP+wXfyo8axMipFwyV9fqesFHF
xRc8j2QHUOVTyqAA9TdksSL1+t8h7a6PmEL6ucIUyGIfcBZaY5kw9/IOQxac8i0W
ZxAElSzFSL8xpimSehZCKLHTF4fTlypSA1srVOb2py6vjOQK+7PZAnlP42TXEgBT
gk6HnAFQBJ6cx8KckYHdqQFPtTKLyglSFLeOCBfbwymOBZgjEopdzTDsE17Z07m8
ZQq08jiSPJKGmwiLI7SCMlA5DMI6B9OV2/h8GXfxP833oR8Z8wQrvzQkM0GnJU5w
Hirk8ECEJndBuLyyVl0suzabNUNdzVWvXQHMIP2+0aW/secAiUbkVdF7o0B5l8qI
GZaHFCZ37BdKgppjApCCLmA/1Th2hHKEwGsIWGiBMQhWfKeRzXxuUZYqSF+YZDRN
ju5ZhJOlmKiLv0AyuMPw9T25X8fTzBwAppjL7r6HM610rNOM0izYz8+7K0ckq74/
jfbt+FuKxGwp8Phz7mm7APOsDsrhNkQgmn1WpnqgJBQwXTRZGzn2ne9npvmUAo3C
4Hiaib2fTIq2Q39scT51b5enjtkPa9S2o4MF+32JP6g5UmLMe3GMpa5V2itvCuN3
ue8AzgNoeyd3oOlqPh9XJi/Kk68it4UacXRkiZAJ9cLTEMixVUjuaVkCyFxsmKxb
vvNa2Of440JgSc2Hw21p
=vdZZ
-----END PGP SIGNATURE-----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Geoghegan wrote:
Couldn't we adopt
AssertVariableIsOfType()/AssertVariableIsOfTypeMacro() to macros like
READ_UINT_FIELD()?I'm surprised that this stuff was only ever used for logical decoding
infrastructure so far.
The reason it's only used there is that Andres is the one who introduced
those macros precisely for that code. We've not yet had time to adjust
the rest of the code to have more sanity checks.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Aug 03, 2015 at 09:57:52AM -0700, Joe Conway wrote:
On 08/03/2015 09:55 AM, Stephen Frost wrote:
* Noah Misch (noah@leadboat.com) wrote:
On Sun, Aug 02, 2015 at 11:31:16PM -0400, Tom Lane wrote:
That being the case, it would probably be a good idea to get
them done before alpha2, as there may not be a good opportunity
afterwards.Freedom to bump catversion after alpha2 will be
barely-distinguishable from freedom to do so now. I have planned
to leave my usual comment period of a few days, though skipping
that would be rather innocuous in this case.This is clearly necessary, of course, and I wouldn't be surprised
if we have another necessary bump post-alpha2, but at the same
time, it'd certainly be nice if we are able to put out alpha2 and
then a beta1 in the future without needing to do a bump.In other words, +1 from me for going ahead and committing this for
alpha2, if possible.+1
Rather than commit on an emergency basis in the few hours between these +1's
and the wrap, I kept to my original schedule. FYI, if it hadn't required
emergency procedures (cancelling the day's plans so I could get to a notebook
computer), I would have committed early based on the three +1s.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Noah,
* Noah Misch (noah@leadboat.com) wrote:
Rather than commit on an emergency basis in the few hours between these +1's
and the wrap, I kept to my original schedule. FYI, if it hadn't required
emergency procedures (cancelling the day's plans so I could get to a notebook
computer), I would have committed early based on the three +1s.
No worries, fully agreed that this was not an emergency in any way.
Thanks!
Stephen
On Mon, Aug 3, 2015 at 10:00 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Couldn't we adopt
AssertVariableIsOfType()/AssertVariableIsOfTypeMacro() to macros like
READ_UINT_FIELD()?I'm surprised that this stuff was only ever used for logical decoding
infrastructure so far.The reason it's only used there is that Andres is the one who introduced
those macros precisely for that code. We've not yet had time to adjust
the rest of the code to have more sanity checks.
Any chance of picking this up, Andres?
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers