Re: [HACKERS] replicating DROP commands across servers
Jeff Janes wrote: On Fri, Jan 2, 2015 at 9:59 PM, Jeff Janes jeff.ja...@gmail.com wrote: This commit 3f88672a4e4d8e648d24ccc65 seems to have broken pg_upgrade for pg_trgm. It seems I failed to notice the get_object_address in the ALTER EXTENSION path. Should be fixed now. I looked for similar missed callsites and didn't find anything. Thanks for the report! -- Álvaro Herrerahttp://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
Re: [HACKERS] replicating DROP commands across servers
On Fri, Jan 2, 2015 at 9:59 PM, Jeff Janes jeff.ja...@gmail.com wrote:
On Mon, Dec 29, 2014 at 2:15 PM, Alvaro Herrera alvhe...@2ndquadrant.com
wrote:
Here's a patch that tweaks the grammar to use TypeName in COMMENT,
SECURITY LABEL, and DROP for the type and domain cases. The required
changes in the code are pretty minimal, thankfully. Note the slight
changes to the new object_address test also.
I think I'm pretty much done with this now, so I intend to push this
first thing tomorrow and then the rebased getObjectIdentityParts patch
sometime later.
This commit 3f88672a4e4d8e648d24ccc65 seems to have broken pg_upgrade for
pg_trgm.
On 9.2.9 freshly initdb and started with default config:
$ createdb jjanes
in psql:
create extension pg_trgm ;
create table foo (x text);
create index on foo using gin (upper(x) gin_trgm_ops);
Then run 9.5's pg_upgrade and it fails at the stage of restoring the
database schema.
The problem also occurs doing a self-upgrade from 9.5 to 9.5.
The contents of the dump not changed meaningfully between 9.4 and 9.5.
I've isolated the problem to the backend applying the pg_restore of the
dump, regardless of which version created the dump.
After compiling 3c9e4cdbf2ec876dbb7 with CFLAGS=-fno-omit-frame-pointer,
I get this backtrace for the core-dump of postmaster during the pg_restore:
Core was generated by `postgres: jjanes jjanes [local] ALTER EXTENSION
'.
Program terminated with signal 11, Segmentation fault.
#0 0x005135ff in NameListToString (names=0x257fcf8) at
namespace.c:2935
2935if (IsA(name, String))
(gdb) bt
#0 0x005135ff in NameListToString (names=0x257fcf8) at
namespace.c:2935
#1 0x00512f33 in DeconstructQualifiedName (names=0x257fcf8,
nspname_p=0x7fff419bc960, objname_p=0x7fff419bc958) at namespace.c:2648
#2 0x0058a746 in LookupTypeName (pstate=0x0, typeName=0x257fd10,
typmod_p=0x0, missing_ok=0 '\000') at parse_type.c:153
#3 0x005220b4 in get_object_address_type (objtype=OBJECT_TYPE,
objname=0x257fd50, missing_ok=0 '\000') at objectaddress.c:1306
#4 0x00520cf5 in get_object_address (objtype=OBJECT_TYPE,
objname=0x257fd50, objargs=0x0, relp=0x7fff419bcb58, lockmode=4,
missing_ok=0 '\000')
at objectaddress.c:678
#5 0x005c0f36 in ExecAlterExtensionContentsStmt (stmt=0x257fd80)
at extension.c:2906
#6 0x0077508c in ProcessUtilitySlow (parsetree=0x257fd80,
queryString=0x254f990 \n-- For binary upgrade, must preserve pg_type
oid\nSELECT
binary_upgrade.set_next_pg_type_oid('16394'::pg_catalog.oid);\n\n\n-- For
binary upgrade, must preserve pg_type array oid\nSELECT binary_upgrade.se...,
context=PROCESS_UTILITY_QUERY, params=0x0, dest=0x2581b60,
completionTag=0x7fff419bd100 ) at utility.c:1167
#7 0x0077490e in standard_ProcessUtility (parsetree=0x257fd80,
queryString=0x254f990 \n-- For binary upgrade, must preserve pg_type
oid\nSELECT
binary_upgrade.set_next_pg_type_oid('16394'::pg_catalog.oid);\n\n\n-- For
binary upgrade, must preserve pg_type array oid\nSELECT binary_upgrade.se...,
context=PROCESS_UTILITY_QUERY, params=0x0, dest=0x2581b60,
completionTag=0x7fff419bd100 ) at utility.c:840
#8 0x00773bcc in ProcessUtility (parsetree=0x257fd80,
queryString=0x254f990 \n-- For binary upgrade, must preserve pg_type
oid\nSELECT
binary_upgrade.set_next_pg_type_oid('16394'::pg_catalog.oid);\n\n\n-- For
binary upgrade, must preserve pg_type array oid\nSELECT binary_upgrade.se...,
context=PROCESS_UTILITY_QUERY, params=0x0, dest=0x2581b60,
completionTag=0x7fff419bd100 ) at utility.c:313
#9 0x00772dd6 in PortalRunUtility (portal=0x2505b90,
utilityStmt=0x257fd80, isTopLevel=0 '\000', dest=0x2581b60,
completionTag=0x7fff419bd100 )
at pquery.c:1187
#10 0x00772f8c in PortalRunMulti (portal=0x2505b90, isTopLevel=0
'\000', dest=0x2581b60, altdest=0x2581b60, completionTag=0x7fff419bd100 )
at pquery.c:1318
#11 0x00772560 in PortalRun (portal=0x2505b90,
count=9223372036854775807, isTopLevel=0 '\000', dest=0x2581b60,
altdest=0x2581b60,
completionTag=0x7fff419bd100 ) at pquery.c:816
#12 0x0076c868 in exec_simple_query (
query_string=0x254f990 \n-- For binary upgrade, must preserve pg_type
oid\nSELECT
binary_upgrade.set_next_pg_type_oid('16394'::pg_catalog.oid);\n\n\n-- For
binary upgrade, must preserve pg_type array oid\nSELECT binary_upgrade.se...)
at postgres.c:1045
#13 0x007708a2 in PostgresMain (argc=1, argv=0x24ed5e0,
dbname=0x24ed4b8 jjanes, username=0x24ed4a0 jjanes) at postgres.c:4012
#14 0x00701940 in BackendRun (port=0x250c1b0) at postmaster.c:4130
#15 0x00701083 in BackendStartup (port=0x250c1b0) at
postmaster.c:3805
#16 0x006fd8c5 in ServerLoop () at postmaster.c:1573
#17 0x006fd013 in PostmasterMain (argc=18, argv=0x24ec480) at
postmaster.c:1220
#18 0x0066476b in main (argc=18, argv=0x24ec480) at
Re: [HACKERS] replicating DROP commands across servers
On Mon, Dec 29, 2014 at 2:15 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's a patch that tweaks the grammar to use TypeName in COMMENT, SECURITY LABEL, and DROP for the type and domain cases. The required changes in the code are pretty minimal, thankfully. Note the slight changes to the new object_address test also. I think I'm pretty much done with this now, so I intend to push this first thing tomorrow and then the rebased getObjectIdentityParts patch sometime later. This commit 3f88672a4e4d8e648d24ccc65 seems to have broken pg_upgrade for pg_trgm. On 9.2.9 freshly initdb and started with default config: $ createdb jjanes in psql: create extension pg_trgm ; create table foo (x text); create index on foo using gin (upper(x) gin_trgm_ops); Then run 9.5's pg_upgrade and it fails at the stage of restoring the database schema. The relevant log files are attached Cheers, Jeff pg_upgrade_server.log Description: Binary data pg_upgrade_dump_16384.log Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Alvaro Herrera wrote:
Patch 0005 adds getObjectIdentityParts(), which returns the object
identity in arrays that can be passed to pg_get_object_address. This
part needs slight revisions so I'm not sure I will be able to push
tomorrow.
Here's a fresh version of this patch. I chose to add a SQL-accessible
version, pg_identify_object_as_address, to make it easier to test. In
doing so I noticed a couple of bugs, and most interestingly I noticed
that it was essentially impossible to cleanly address an array type;
doing a roundtrip through the new functions would get me the base type
when I used integer[] but the array type when I used _int4. This
looked like a problem, so I traced through it and noticed that we're
using the type name *list* as a list, rather than as a TypeName, to
refer to OBJECT_TYPE and OBJECT_DOMAIN; I hadn't understood the
significance of this until I realized that domains would be represented
with arrayBounds set to a non-empty list for the integer[] syntax, but
the name list would have pg_catalog integer only; when the rest of
TypeName was discarded, the fact that we were talking about an array was
completely forgotten. Before the dawn of time we had this:
-static void
-CommentType(List *typename, char *comment)
-{
- TypeName *tname;
- Oid oid;
-
- /* XXX a bit of a crock; should accept TypeName in COMMENT syntax */
- tname = makeTypeNameFromNameList(typename);
where the XXX comment was removed by commit c10575ff005c330d047534562
without a corresponding comment in the new function.
I'm going to see about changing the grammar to get this fixed; this
patch is important because it will enable us to complete^Wcontinue
working on the DDL deparse testing framework.
--
Álvaro Herrerahttp://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
Re: [HACKERS] replicating DROP commands across servers
(The changes in the regression test are bogus, BTW; I didn't care enough
to get them fixed before sorting out the rest of the mess.)
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
commit 89c8cbed0072ad4d921128b834fcb4f9e2eb4c33
Author: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Mon Dec 22 18:32:43 2014 -0300
array objname/objargs stuff
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 24c64b7..112b6a0 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17772,6 +17772,23 @@ FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
identifier present in the identity is quoted if necessary.
/entry
/row
+ row
+entryliteraladdress_names/literal/entry
+entrytypetext[]/type/entry
+entry
+ An array that, together with literaladdress_args/literal,
+ can be used by the functionpg_get_object_address()/function to
+ recreate the object address in a remote server containing an
+ identically named object of the same kind.
+/entry
+ /row
+ row
+entryliteraladdress_args/literal/entry
+entrytypetext[]/type/entry
+entry
+ Complement for literaladdress_names/literal above.
+/entry
+ /row
/tbody
/tgroup
/informaltable
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 85079d6..789af5f 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -74,6 +74,7 @@
#include utils/builtins.h
#include utils/fmgroids.h
#include utils/lsyscache.h
+#include utils/memutils.h
#include utils/syscache.h
#include utils/tqual.h
@@ -556,8 +557,9 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid,
int32 objectSubId);
static void getProcedureTypeDescription(StringInfo buffer, Oid procid);
static void getConstraintTypeDescription(StringInfo buffer, Oid constroid);
-static void getOpFamilyIdentity(StringInfo buffer, Oid opfid);
-static void getRelationIdentity(StringInfo buffer, Oid relid);
+static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname,
+ List **objargs);
+static void getRelationIdentity(StringInfo buffer, Oid relid, List **objname);
/*
* Translate an object name and arguments (as passed by the parser) to an
@@ -2960,6 +2962,66 @@ pg_identify_object(PG_FUNCTION_ARGS)
}
/*
+ * SQL-level callable function to obtain object type + identity
+ */
+Datum
+pg_identify_object_as_address(PG_FUNCTION_ARGS)
+{
+ Oid classid = PG_GETARG_OID(0);
+ Oid objid = PG_GETARG_OID(1);
+ int32 subobjid = PG_GETARG_INT32(2);
+ ObjectAddress address;
+ char *identity;
+ List *names;
+ List *args;
+ Datum values[3];
+ bool nulls[3];
+ TupleDesc tupdesc;
+ HeapTuple htup;
+
+ address.classId = classid;
+ address.objectId = objid;
+ address.objectSubId = subobjid;
+
+ /*
+ * Construct a tuple descriptor for the result row. This must match this
+ * function's pg_proc entry!
+ */
+ tupdesc = CreateTemplateTupleDesc(3, false);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 1, type,
+ TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 2, object_names,
+ TEXTARRAYOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 3, object_args,
+ TEXTARRAYOID, -1, 0);
+
+ tupdesc = BlessTupleDesc(tupdesc);
+
+ /* object type */
+ values[0] = CStringGetTextDatum(getObjectTypeDescription(address));
+ nulls[0] = false;
+
+ /* object identity */
+ identity = getObjectIdentityParts(address, names, args);
+ pfree(identity);
+
+ /* object_names */
+ values[1] = PointerGetDatum(strlist_to_textarray(names));
+ nulls[1] = false;
+
+ /* object_args */
+ if (args)
+ values[2] = PointerGetDatum(strlist_to_textarray(args));
+ else
+ values[2] = PointerGetDatum(construct_empty_array(TEXTOID));
+ nulls[2] = false;
+
+ htup = heap_form_tuple(tupdesc, values, nulls);
+
+ PG_RETURN_DATUM(HeapTupleGetDatum(htup));
+}
+
+/*
* Return a palloc'ed string that describes the type of object that the
* passed address is for.
*
@@ -3215,7 +3277,7 @@ getProcedureTypeDescription(StringInfo buffer, Oid procid)
}
/*
- * Return a palloc'ed string that identifies an object.
+ * Obtain a given object's identity, as a palloc'ed string.
*
* This is for machine consumption, so it's not translated. All elements are
* schema-qualified when appropriate.
@@ -3223,14 +3285,42 @@ getProcedureTypeDescription(StringInfo buffer, Oid procid)
char *
getObjectIdentity(const ObjectAddress *object)
{
+ return getObjectIdentityParts(object, NULL, NULL);
+}
+
+/*
+ * As above, but more detailed.
+ *
+ * There are two sets of return values: the identity itself as a palloc'd
+ * string is returned. objname and objargs, if not NULL, are output parameters
+ * that receive lists of C-strings that are useful to give
Re: [HACKERS] replicating DROP commands across servers
Here's a patch that tweaks the grammar to use TypeName in COMMENT,
SECURITY LABEL, and DROP for the type and domain cases. The required
changes in the code are pretty minimal, thankfully. Note the slight
changes to the new object_address test also.
I think I'm pretty much done with this now, so I intend to push this
first thing tomorrow and then the rebased getObjectIdentityParts patch
sometime later.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
commit 701a2b7cc3cc6dff865db64ecaeb2fd8bb07b396
Author: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Mon Dec 29 18:55:59 2014 -0300
Use TypeName to represent type names in certain commands
In COMMENT, DROP and SECURITY LABEL we were representing types as a list
of names, rather than the TypeName struct that's used everywhere; this
practice also recently found its way into the new pg_get_object_address.
Right now it doesn't affect anything (other than some code cleanliness),
but it is more problematic for future changes that operate with object
addresses from the SQL interface; type details such as array-ness were
being forgotten.
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 7cb46e1..9ca609d 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -646,13 +646,11 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
break;
case OBJECT_DOMCONSTRAINT:
{
- List *domname;
ObjectAddress domaddr;
char *constrname;
- domname = list_truncate(list_copy(objname), list_length(objname) - 1);
- constrname = strVal(llast(objname));
- domaddr = get_object_address_type(OBJECT_DOMAIN, domname, missing_ok);
+ domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok);
+ constrname = strVal(linitial(objargs));
address.classId = ConstraintRelationId;
address.objectId = get_domain_constraint_oid(domaddr.objectId,
@@ -1291,14 +1289,13 @@ get_object_address_attrdef(ObjectType objtype, List *objname,
* Find the ObjectAddress for a type or domain
*/
static ObjectAddress
-get_object_address_type(ObjectType objtype,
- List *objname, bool missing_ok)
+get_object_address_type(ObjectType objtype, List *objname, bool missing_ok)
{
ObjectAddress address;
TypeName *typename;
Type tup;
- typename = makeTypeNameFromNameList(objname);
+ typename = (TypeName *) linitial(objname);
address.classId = TypeRelationId;
address.objectId = InvalidOid;
@@ -1428,27 +1425,8 @@ pg_get_object_address(PG_FUNCTION_ARGS)
* given object type. Most use a simple string Values list, but there
* are some exceptions.
*/
- if (type == OBJECT_TYPE || type == OBJECT_DOMAIN)
- {
- Datum *elems;
- bool *nulls;
- int nelems;
- TypeName *typname;
-
- deconstruct_array(namearr, TEXTOID, -1, false, 'i',
- elems, nulls, nelems);
- if (nelems != 1)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg(name list length must be exactly %d, 1)));
- if (nulls[0])
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg(name or argument lists may not contain nulls)));
- typname = typeStringToTypeName(TextDatumGetCString(elems[0]));
- name = typname-names;
- }
- else if (type == OBJECT_CAST)
+ if (type == OBJECT_TYPE || type == OBJECT_DOMAIN || type == OBJECT_CAST ||
+ type == OBJECT_DOMCONSTRAINT)
{
Datum *elems;
bool *nulls;
@@ -1533,18 +1511,13 @@ pg_get_object_address(PG_FUNCTION_ARGS)
*/
switch (type)
{
- case OBJECT_DOMCONSTRAINT:
- if (list_length(name) 2)
-ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg(name list length must be at least %d, 2)));
- break;
case OBJECT_LARGEOBJECT:
if (list_length(name) != 1)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg(name list length must be %d, 1)));
+ errmsg(name list length must be exactly %d, 1)));
break;
+ case OBJECT_DOMCONSTRAINT:
case OBJECT_OPCLASS:
case OBJECT_OPFAMILY:
case OBJECT_CAST:
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index 8583581..21a2ae4 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -264,10 +264,14 @@ does_not_exist_skipping(ObjectType objtype, List *objname, List *objargs)
{
case OBJECT_TYPE:
case OBJECT_DOMAIN:
- if (!schema_does_not_exist_skipping(objname, msg, name))
{
-msg = gettext_noop(type \%s\ does not exist, skipping);
-name = TypeNameToString(makeTypeNameFromNameList(objname));
+TypeName *typ = linitial(objname);
+
+if (!schema_does_not_exist_skipping(typ-names, msg, name))
+{
+ msg = gettext_noop(type \%s\ does not exist, skipping);
+ name = TypeNameToString(typ);
+}
}
break;
case
Re: [HACKERS] replicating DROP commands across servers
On 2014-12-24 21:54:20 +1300, David Rowley wrote: On 24 December 2014 at 07:41, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I have pushed patches 0001 through 0004, with some revisions. Only the final part is missing. Hi Alvaro, Would you be able to commit the attached? It just fixes a new compiler warning that I'm seeing on MSVC. src\backend\parser\parse_type.c(795): warning C4715: 'typeStringToTypeName' : not all control paths return a value [D:\Postgres\a\postgres.vcxproj] Pushed. I really wonder if we can't make msvc reliably recognize this kind of scenario - especially this case is pretty trivial? Which of: #if defined(HAVE__BUILTIN_UNREACHABLE) !defined(USE_ASSERT_CHECKING) #define pg_unreachable() __builtin_unreachable() #elif defined(_MSC_VER) !defined(USE_ASSERT_CHECKING) #define pg_unreachable() __assume(0) #else #define pg_unreachable() abort() #endif does your build end up using? Does changing things around make it recognize this and similar cases? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
On 25 December 2014 at 00:34, Andres Freund and...@2ndquadrant.com wrote:
On 2014-12-24 21:54:20 +1300, David Rowley wrote:
On 24 December 2014 at 07:41, Alvaro Herrera alvhe...@2ndquadrant.com
wrote:
I have pushed patches 0001 through 0004, with some revisions. Only the
final part is missing.
Hi Alvaro,
Would you be able to commit the attached? It just fixes a new compiler
warning that I'm seeing on MSVC.
src\backend\parser\parse_type.c(795): warning C4715:
'typeStringToTypeName'
: not all control paths return a value [D:\Postgres\a\postgres.vcxproj]
Pushed.
Thanks
I really wonder if we can't make msvc reliably recognize this kind of
scenario - especially this case is pretty trivial?
Which of:
#if defined(HAVE__BUILTIN_UNREACHABLE) !defined(USE_ASSERT_CHECKING)
#define pg_unreachable() __builtin_unreachable()
#elif defined(_MSC_VER) !defined(USE_ASSERT_CHECKING)
#define pg_unreachable() __assume(0)
#else
#define pg_unreachable() abort()
#endif
I don't think the problem is here. The problem is the the elevel being set
to a variable in the elog macro to prevent the multiple evaluation problem,
then since it does int elevel_ = elevel ... if (elevel_ = ERROR) that's
not constant, or at least the microsoft compiler is not smart enough to see
that it is.
The attached patch removes the warning, but likely can't be used in case
someone somewhere is doing elog(var++, my error);
Compiling with the attached shaves almost 1% off the size of postgres.exe:
before; 5,882,368 bytes
after: 5,830,656 bytes
I've been trawling around to try to see if anything
like __builtin_constant_p() exists for MSVC, but so far I've not found
anything useful.
Kind Regards
David Rowley
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 87438b8..8be68dd 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -121,10 +121,9 @@
#else /*
!HAVE__BUILTIN_CONSTANT_P */
#define ereport_domain(elevel, domain, rest) \
do { \
- const int elevel_ = (elevel); \
- if (errstart(elevel_, __FILE__, __LINE__, PG_FUNCNAME_MACRO,
domain)) \
+ if (errstart(elevel, __FILE__, __LINE__, PG_FUNCNAME_MACRO,
domain)) \
errfinish rest; \
- if (elevel_ = ERROR) \
+ if (elevel = ERROR) \
pg_unreachable(); \
} while(0)
#endif /* HAVE__BUILTIN_CONSTANT_P */
@@ -258,12 +257,10 @@ extern intgetinternalerrposition(void);
#else /*
!HAVE__BUILTIN_CONSTANT_P */
#define elog(elevel, ...) \
do { \
- int elevel_; \
- elog_start(__FILE__, __LINE__, PG_FUNCNAME_MACRO); \
- elevel_ = (elevel); \
- elog_finish(elevel_, __VA_ARGS__); \
- if (elevel_ = ERROR) \
- pg_unreachable(); \
+ elog_start(__FILE__, __LINE__, PG_FUNCNAME_MACRO); \
+ elog_finish(elevel, __VA_ARGS__); \
+ if (elevel = ERROR) \
+ pg_unreachable(); \
} while(0)
#endif /* HAVE__BUILTIN_CONSTANT_P */
#else /* !HAVE__VA_ARGS */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Andres Freund and...@2ndquadrant.com writes: I really wonder if we can't make msvc reliably recognize this kind of scenario - especially this case is pretty trivial? Even if MSVC did understand pg_unreachable(), there would be other compilers that didn't, so we'd need to worry about suppressing such warnings anyhow. Personally I'm just as happy that we have instances of this case in the buildfarm where we can check it easily. 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
Re: [HACKERS] replicating DROP commands across servers
David Rowley dgrow...@gmail.com writes: On 25 December 2014 at 00:34, Andres Freund and...@2ndquadrant.com wrote: I really wonder if we can't make msvc reliably recognize this kind of scenario - especially this case is pretty trivial? The attached patch removes the warning, but likely can't be used in case someone somewhere is doing elog(var++, my error); Yeah, we're *not* doing that. There are definitely places where ereport/elog are used with nonconstant elevel. It's curious though that MSVC fails to notice that the variable never changes. I wonder whether we could get away with changing the elog macro to do const int elevel_ = (elevel); as ereport does, and whether it would help if so. 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
Re: [HACKERS] replicating DROP commands across servers
On 25 December 2014 at 04:47, Tom Lane t...@sss.pgh.pa.us wrote: David Rowley dgrow...@gmail.com writes: On 25 December 2014 at 00:34, Andres Freund and...@2ndquadrant.com wrote: I really wonder if we can't make msvc reliably recognize this kind of scenario - especially this case is pretty trivial? The attached patch removes the warning, but likely can't be used in case someone somewhere is doing elog(var++, my error); Yeah, we're *not* doing that. There are definitely places where ereport/elog are used with nonconstant elevel. Agreed. The patch was intended as a demo of where the problem is. Although I don't see why non-const elevel matters. Non-stable expressions are what actually matter. It's curious though that MSVC fails to notice that the variable never changes. I wonder whether we could get away with changing the elog macro to do const int elevel_ = (elevel); as ereport does, and whether it would help if so. Unlikely, as the one that was just fixed above is an ereport. I'll dig around a little more and see if there's some way to get MSVC to optimise this somehow. The 1% reduction in the postgres.exe seems worth a little bit of investigation time. Regards David Rowley
Re: [HACKERS] replicating DROP commands across servers
I have pushed patches 0001 through 0004, with some revisions. Only the final part is missing. -- Álvaro Herrerahttp://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
Re: [HACKERS] replicating DROP commands across servers
Here's a five-part split of the remaining pieces of this patch. Patch 0001 is the one I posted in http://www.postgresql.org/message-id/20141220022308.gy1...@alvh.no-ip.org which adds support for COMMENT ON CONSTRAINT .. ON DOMAIN. This just splits OBJECT_CONSTRAINT in OBJECT_TABCONSTRAINT and OBJECT_DOMCONSTRAINT. It includes \dd support and pg_dump support for comments on domain constraint comments. I intend to commit this one first thing tomorrow. Patch 0002 adds OBJECT_DEFAULT support. This is not needed currently, so there's no bug being fixed; we just need it if we want to use get_object_address in a way different from currently. Patch 0003 adds an (unused) table and routine to map the strings returned by getObjectTypeDescription into enum ObjectType, for use of 0004. It also splits a part of parseTypeString into a new function typeStringToTypeName(), for use of 0004. Patch 0004 adds a SQL-callable interface to get_object_address, imaginatively called pg_get_object_address; this uses the stuff in patch 0003. It includes a simple regression test. The code that prepares from text arrays into the appropriate List structure is messy because it needs to mimic parser output. I intend to push these three patches as a single commit tomorrow. Patch 0005 adds getObjectIdentityParts(), which returns the object identity in arrays that can be passed to pg_get_object_address. This part needs slight revisions so I'm not sure I will be able to push tomorrow. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services From f5a324e864baf60df989e313740744732c04404d Mon Sep 17 00:00:00 2001 From: Alvaro Herrera alvhe...@alvh.no-ip.org Date: Fri, 19 Dec 2014 17:03:29 -0300 Subject: [PATCH 1/5] Distinguish domain constraint from table constraints --- doc/src/sgml/ref/comment.sgml | 14 ++ src/backend/catalog/objectaddress.c| 26 ++ src/backend/commands/alter.c | 3 ++- src/backend/commands/event_trigger.c | 3 ++- src/backend/commands/tablecmds.c | 2 +- src/backend/parser/gram.y | 18 +- src/backend/parser/parse_utilcmd.c | 2 +- src/backend/tcop/utility.c | 5 ++--- src/bin/pg_dump/pg_dump.c | 17 + src/bin/psql/describe.c| 27 +-- src/include/nodes/parsenodes.h | 3 ++- src/test/regress/input/constraints.source | 21 + src/test/regress/output/constraints.source | 19 +++ 13 files changed, 141 insertions(+), 19 deletions(-) diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index 36a7312..62e1968 100644 --- a/doc/src/sgml/ref/comment.sgml +++ b/doc/src/sgml/ref/comment.sgml @@ -28,6 +28,7 @@ COMMENT ON COLLATION replaceable class=PARAMETERobject_name/replaceable | COLUMN replaceable class=PARAMETERrelation_name/replaceable.replaceable class=PARAMETERcolumn_name/replaceable | CONSTRAINT replaceable class=PARAMETERconstraint_name/replaceable ON replaceable class=PARAMETERtable_name/replaceable | + CONSTRAINT replaceable class=PARAMETERconstraint_name/replaceable ON DOMAIN replaceable class=PARAMETERdomain_name/replaceable | CONVERSION replaceable class=PARAMETERobject_name/replaceable | DATABASE replaceable class=PARAMETERobject_name/replaceable | DOMAIN replaceable class=PARAMETERobject_name/replaceable | @@ -127,6 +128,18 @@ COMMENT ON /varlistentry varlistentry +termreplaceable class=parametertable_name/replaceable/term +termreplaceable class=parameterdomain_name/replaceable/term +listitem + para + When creating a comment on a constraint on a table or a domain, these + parameteres specify the name of the table or domain on which the + constraint is defined. + /para +/listitem + /varlistentry + + varlistentry termreplaceablesource_type/replaceable/term listitem para @@ -266,6 +279,7 @@ COMMENT ON COLLATION fr_CA IS 'Canadian French'; COMMENT ON COLUMN my_table.my_column IS 'Employee ID number'; COMMENT ON CONVERSION my_conv IS 'Conversion to UTF8'; COMMENT ON CONSTRAINT bar_col_cons ON bar IS 'Constrains column col'; +COMMENT ON CONSTRAINT dom_col_constr ON DOMAIN dom IS 'Constrains col of domain'; COMMENT ON DATABASE my_database IS 'Development Database'; COMMENT ON DOMAIN my_domain IS 'Email Address Domain'; COMMENT ON EXTENSION hstore IS 'implements the hstore data type'; diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index e261307..297deb5 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -530,11 +530,28 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, break; case OBJECT_RULE: case OBJECT_TRIGGER: - case
Re: [HACKERS] replicating DROP commands across servers
Alvaro Herrera wrote: Alvaro Herrera wrote: Andres Freund wrote: Having reread the patch just now I basically see two things to criticize: a) why isn't this accessible at SQL level? That seems easy to address. b) Arguably some of this could well be done in separate commits. Fair comments. I will split it up. Here's a split version. The last part is still missing some polish -- in particular handling for OBJECT_POLICY, and the SQL interface which would also allow us to get something in the regression tests. Pushed patch 1. -- Álvaro Herrerahttp://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
Re: [HACKERS] replicating DROP commands across servers
Michael Paquier wrote: This patch has had no activity for the last two months, is in Needs Review state and has marked as reviewer Dimitri. As there is no activity from the reviewer, I am moving that to CF 2014-12 and removing Dimitri as reviewer. If someone wants to have a look at this patch, feel free to do so. Dimitri, if you are still planning to look at it, please re-add your name. FWIW I intend to commit the first patch more or less as-is, and then add a SQL function accessor to get_object_address to the second part and commit that one also. -- Álvaro Herrerahttp://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
Re: [HACKERS] replicating DROP commands across servers
On 2014-10-04 21:12:24 -0400, Robert Haas wrote: On Fri, Oct 3, 2014 at 4:58 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: I'm not really very convinced that it's a good idea to expose this instead of just figuring out a way to parse the object identity. That's the first thing I tried. But it's not pretty: you have to extract schema names by splitting at a period (and what if a schema name contains a period?), Please tell me that the literals are escaped if necessary. If so, this is pretty easy. quote_literal() is not a hard transformation to reverse, and splitting on a unquoted period is not hard... split out on ON for certain object types, ...nor is splitting on any other fixed text string, such as ON . I'm not following here. Maybe just because I'm misunderstanding your position. The patch imo consists out of the following parts: 1) Addition of dependency information to the dropped object list 2) Actual get_object_address() handling for default values - the current behaviour looks pretty borked to me. 3) The reverse of getObjectTypeDescription() 4) getObjectIdentityParts() - a slightly more detailed version of getObjectIdentity() that requires less string parsing 5) drop even trigger support for a few types. Are you saying you want to add a function to do 4) via parsing inside postgres or are you suggesting to do that in every user of this facility? If the former, why would it be preferrable to do string parsing if we have access to the source data? That part of the patch looks trivial to me? If the latter, I don't see the advantage either - this is complicated enough, why should different users repeat the work? Am I misunderstanding you here? Having reread the patch just now I basically see two things to criticize: a) why isn't this accessible at SQL level? That seems easy to address. b) Arguably some of this could well be done in separate commits. It's just not sane to try to parse such text strings. But this is a pretty ridiculous argument. We have an existing parser that does it just fine Which happens to be the part of postgres that's copied most often. So it's certainly not something appearing to be trivial. ,and a special-purpose parser that does just that (and not all of the other stuff that the main parser does) would be a great deal simpler. That parser also happens to be far from trivial (if we're talking about parseNameAndArgTypes() - which just solves one of the cases. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
Andres Freund wrote: Having reread the patch just now I basically see two things to criticize: a) why isn't this accessible at SQL level? That seems easy to address. b) Arguably some of this could well be done in separate commits. Fair comments. I will split it up. FWIW, I spent some time today fighting with this stuff but the OBJECT_POLICY stuff has been causing me some trouble. I'm not sure that what's there currently in get_object_address is completely sane -- for one thing I'm unsure that tables are the only object kind in the system that are subject to policies. In that case, we have a shortage of abstraction here, it seems, which will need some additional fixes. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
Alvaro Herrera wrote:
Andres Freund wrote:
Having reread the patch just now I basically see two things to
criticize:
a) why isn't this accessible at SQL level? That seems easy to address.
b) Arguably some of this could well be done in separate commits.
Fair comments. I will split it up.
Here's a split version. The last part is still missing some polish --
in particular handling for OBJECT_POLICY, and the SQL interface which
would also allow us to get something in the regression tests.
Note: in this patch series you can find the ObjectTypeMap thing that you
thought was obsolete in the DDL deparse patch ...
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 92816868c1c717519a76a83e65cd0b48e3726fbf Mon Sep 17 00:00:00 2001
From: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Fri, 4 Apr 2014 16:05:48 -0300
Subject: [PATCH 1/3] add normal/original flags to
pg_event_trigger_dropped_objects
---
doc/src/sgml/func.sgml | 13 ++
src/backend/catalog/dependency.c| 21 ++-
src/backend/commands/event_trigger.c| 16 +---
src/include/catalog/pg_proc.h | 2 +-
src/include/commands/event_trigger.h| 3 ++-
src/test/regress/expected/event_trigger.out | 40 +
src/test/regress/sql/event_trigger.sql | 30 ++
7 files changed, 114 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7e5bcd9..45f3efa 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17583,6 +17583,19 @@ FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
entryObject sub-id (e.g. attribute number for columns)/entry
/row
row
+entryliteraloriginal/literal/entry
+entrytypebool/type/entry
+entryFlag used to identify the root object of the deletion/entry
+ /row
+ row
+entryliteralnormal/literal/entry
+entrytypebool/type/entry
+entry
+ Flag indicating that there's a normal dependency relationship
+ in the dependency graph leading to this object
+/entry
+ /row
+ row
entryliteralobject_type/literal/entry
entrytypetext/type/entry
entryType of the object/entry
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 256486c..6485e3d 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -205,16 +205,25 @@ deleteObjectsInList(ObjectAddresses *targetObjects, Relation *depRel,
/*
* Keep track of objects for event triggers, if necessary.
*/
- if (trackDroppedObjectsNeeded())
+ if (trackDroppedObjectsNeeded() !(flags PERFORM_DELETION_INTERNAL))
{
for (i = 0; i targetObjects-numrefs; i++)
{
- ObjectAddress *thisobj = targetObjects-refs + i;
-
- if ((!(flags PERFORM_DELETION_INTERNAL))
-EventTriggerSupportsObjectClass(getObjectClass(thisobj)))
+ const ObjectAddress *thisobj = targetObjects-refs[i];
+ const ObjectAddressExtra *extra = targetObjects-extras[i];
+ bool original = false;
+ bool normal = false;
+
+ if (extra-flags DEPFLAG_ORIGINAL)
+original = true;
+ if (extra-flags DEPFLAG_NORMAL)
+normal = true;
+ if (extra-flags DEPFLAG_REVERSE)
+normal = true;
+
+ if (EventTriggerSupportsObjectClass(getObjectClass(thisobj)))
{
-EventTriggerSQLDropAddObject(thisobj);
+EventTriggerSQLDropAddObject(thisobj, original, normal);
}
}
}
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 1b8c94b..0bab971 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -112,6 +112,8 @@ typedef struct SQLDropObject
const char *objname;
const char *objidentity;
const char *objecttype;
+ bool original;
+ bool normal;
slist_node next;
} SQLDropObject;
@@ -1105,7 +1107,7 @@ trackDroppedObjectsNeeded(void)
* Register one object as being dropped by the current command.
*/
void
-EventTriggerSQLDropAddObject(ObjectAddress *object)
+EventTriggerSQLDropAddObject(const ObjectAddress *object, bool original, bool normal)
{
SQLDropObject *obj;
MemoryContext oldcxt;
@@ -1124,6 +1126,8 @@ EventTriggerSQLDropAddObject(ObjectAddress *object)
obj = palloc0(sizeof(SQLDropObject));
obj-address = *object;
+ obj-original = original;
+ obj-normal = normal;
/*
* Obtain schema names from the object's catalog tuple, if one exists;
@@ -1251,8 +1255,8 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
{
SQLDropObject *obj;
int i = 0;
- Datum values[7];
- bool nulls[7];
+ Datum values[9];
+ bool nulls[9];
obj = slist_container(SQLDropObject, next, iter.cur);
@@ -1268,6 +1272,12 @@ pg_event_trigger_dropped_objects(PG_FUNCTION_ARGS)
/* objsubid */
values[i++] =
Re: [HACKERS] replicating DROP commands across servers
Robert Haas wrote: On Fri, Oct 3, 2014 at 4:58 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: I'm not really very convinced that it's a good idea to expose this instead of just figuring out a way to parse the object identity. That's the first thing I tried. But it's not pretty: you have to extract schema names by splitting at a period (and what if a schema name contains a period?), Please tell me that the literals are escaped if necessary. If so, this is pretty easy. quote_literal() is not a hard transformation to reverse, and splitting on a unquoted period is not hard... I don't think it is necessary to force parsing strings for something that can be more conveniently obtained from the get go, just because we're too lazy to change the existing definition of the function. I'm not saying it is impossible or extremely hard to parse the strings, but since we can emit the right format with no extra effort, there doesn't seem to be any point on doing it that way. It's not like this patch adds excessive extra complexity to this code, either. I'd say that the most complex part of this patch is the addition of the two flag (normal and original) columns, which we would need regardless of the rest of the patch; these are used to tell whether there are routes to the given object that have the eponymous flags set in the dependency graph. It's just not sane to try to parse such text strings. But this is a pretty ridiculous argument. We have an existing parser that does it just fine, and a special-purpose parser that does just that (and not all of the other stuff that the main parser does) would be a great deal simpler. We have a main parser because we have no other option --- it's the way stuff gets into the system in the first place. But in this case, it's not about accepting communication from the outside world, but emitting state that is already in the database, in a different format. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
On 10/6/14, 11:24 PM, Robert Haas wrote: Offlist. FWIW, I've run into situations more than once in userspace where I need a way to properly separate schema and object name. Generally I can make do using reg* casts and then hitting catalog tables, but it'd be nice if there was an easier way. Sure, although I think that's a bit of a separate problem. It's hard to iterate through a string a character at a time from the SQL level so that you can handle stuff like the quote_literal() rules. If we want people to be able to do that easily we need to provide tools to handle it. But C is actually quite well-suited to such tasks. Yeah, I wouldn't want to attempt this in SQL; I was saying that a built-in function to do this would be broadly useful, not just for replicating DROPs. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Jim Nasby wrote: On 10/6/14, 11:24 PM, Robert Haas wrote: Offlist. FWIW, I've run into situations more than once in userspace where I need a way to properly separate schema and object name. Generally I can make do using reg* casts and then hitting catalog tables, but it'd be nice if there was an easier way. Sure, although I think that's a bit of a separate problem. It's hard to iterate through a string a character at a time from the SQL level so that you can handle stuff like the quote_literal() rules. If we want people to be able to do that easily we need to provide tools to handle it. But C is actually quite well-suited to such tasks. Yeah, I wouldn't want to attempt this in SQL; I was saying that a built-in function to do this would be broadly useful, not just for replicating DROPs. Well, most of what you need is served by pg_identify_object, I think: just grab the OID from the appropriate catalog, and the OID of the catalog itself; that function will give you schema and name, which is what you need. Probably the most difficult part is figuring out which reg* cast you want .. and of course there are also cases where there is no such datatype to cast to in the first place. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
On 10/4/14, 8:12 PM, Robert Haas wrote: It's just not sane to try to parse such text strings. But this is a pretty ridiculous argument. We have an existing parser that does it just fine, and a special-purpose parser that does just that (and not all of the other stuff that the main parser does) would be a great deal simpler. Maybe there are examples other than the ones you listed here that demonstrate that this is actually a hard problem, but the fact that you might need to undo quote_literal() or search for and split on fixed strings does not. FWIW, I've run into situations more than once in userspace where I need a way to properly separate schema and object name. Generally I can make do using reg* casts and then hitting catalog tables, but it'd be nice if there was an easier way. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
On Fri, Oct 3, 2014 at 4:58 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: I'm not really very convinced that it's a good idea to expose this instead of just figuring out a way to parse the object identity. That's the first thing I tried. But it's not pretty: you have to extract schema names by splitting at a period (and what if a schema name contains a period?), Please tell me that the literals are escaped if necessary. If so, this is pretty easy. quote_literal() is not a hard transformation to reverse, and splitting on a unquoted period is not hard... split out on ON for certain object types, ...nor is splitting on any other fixed text string, such as ON . figure out parens and argument types and names for functions and aggregates, etc. I certainly agree that parsing out parens and argument types and names for functions and aggregates is the hardest part of this, mostly because you can't count a comma to mark the end of one argument and the beginning of the next - you have to account for quoted identifiers, and you might be inside a numeric typemod or similar. It's just not sane to try to parse such text strings. But this is a pretty ridiculous argument. We have an existing parser that does it just fine, and a special-purpose parser that does just that (and not all of the other stuff that the main parser does) would be a great deal simpler. Maybe there are examples other than the ones you listed here that demonstrate that this is actually a hard problem, but the fact that you might need to undo quote_literal() or search for and split on fixed strings does not. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
On 09/16/2014 09:09 PM, Brightwell, Adam wrote: I think there's been some changes to this patch since july, care to resend a new version? Sure, here it is. The only difference with the previous version is that it now also supports column defaults. This was found to be a problem when you drop a sequence that some column default depends on -- for example a column declared SERIAL, or a sequence marked with ALTER SEQUENCE OWNED BY. The new code is able to drop both the sequence and the default value (leaving, of course, the rest of the column intact.) This required adding support for such objects in get_object_address. I have given this patch the following review: - Apply to current master (77e65bf). -- success - check-world. --success - multiple FIXME statements still exist -- are there plans to fix these items? Can the duplicated code be extracted to a static function? Nothing seems to be happening to this, so I'm marking this as returned with feedback. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Heikki Linnakangas wrote: On 09/16/2014 09:09 PM, Brightwell, Adam wrote: I have given this patch the following review: - Apply to current master (77e65bf). -- success - check-world. --success - multiple FIXME statements still exist -- are there plans to fix these items? Can the duplicated code be extracted to a static function? Nothing seems to be happening to this, so I'm marking this as returned with feedback. Meh. There are three fixmes in the code. One can be handled by just removing the line; we don't really care about duplicating 10 lines of boilerplate code. The other two mean missing support for domain constraints and for default ACLs. Is there absolutely no feedback to be had on the mechanism used by the patch? Since the patch has had good feedback and no further comments arise, I can just implement support for those two missing object types and push, and everybody will be happy. Right? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: There are three fixmes in the code. One can be handled by just removing the line; we don't really care about duplicating 10 lines of boilerplate code. The other two mean missing support for domain constraints and for default ACLs. Is there absolutely no feedback to be had on the mechanism used by the patch? Since the patch has had good feedback and no further comments arise, I can just implement support for those two missing object types and push, and everybody will be happy. Right? In general, I'd say yes, but I'll take a look at the patch now and provide feedback in a couple hours anyway. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] replicating DROP commands across servers
On 10/03/2014 08:08 PM, Stephen Frost wrote: Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: There are three fixmes in the code. One can be handled by just removing the line; we don't really care about duplicating 10 lines of boilerplate code. The other two mean missing support for domain constraints and for default ACLs. Is there absolutely no feedback to be had on the mechanism used by the patch? Since the patch has had good feedback and no further comments arise, I can just implement support for those two missing object types and push, and everybody will be happy. Right? In general, I'd say yes, but I'll take a look at the patch now and provide feedback in a couple hours anyway. Thanks Stephen! I had a very brief look at the docs, and these extra outputs from pg_event_trigger_dropped_objects caught my eye: +row + entryliteraladdress_names/literal/entry + entrytypetext[]/type/entry + entry + An array that, together with literaladdress_args/literal, + can be used by the C-language function getObjectAddress() to + recreate the object address in a remote server containing a similar object. + /entry +/row +row + entryliteraladdress_args/literal/entry + entrytypetext[]/type/entry + entry + See literaladdress_names/literal above. + /entry +/row I couldn't find a function called getObjectAddress anywhere. Typo? Also, is providing a C-language function the best we can do? The rest of the information returned by pg_event_trigger_dropped_objects is usable from any language. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Heikki Linnakangas wrote: I had a very brief look at the docs, and these extra outputs from pg_event_trigger_dropped_objects caught my eye: +row + entryliteraladdress_names/literal/entry + entrytypetext[]/type/entry + entry + An array that, together with literaladdress_args/literal, + can be used by the C-language function getObjectAddress() to + recreate the object address in a remote server containing a similar object. + /entry +/row +row + entryliteraladdress_args/literal/entry + entrytypetext[]/type/entry + entry + See literaladdress_names/literal above. + /entry +/row I couldn't find a function called getObjectAddress anywhere. Typo? Ah, yeah, it's get_object_address actually. Also, is providing a C-language function the best we can do? The rest of the information returned by pg_event_trigger_dropped_objects is usable from any language. Well, the return value from get_object_address is an ObjectAddress. It's simple enough to create an SQL wrapper that takes the address_names/address_args arrays and return an ObjectAddress; would this be useful? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
On 10/03/2014 09:06 PM, Alvaro Herrera wrote: Heikki Linnakangas wrote: I had a very brief look at the docs, and these extra outputs from pg_event_trigger_dropped_objects caught my eye: +row + entryliteraladdress_names/literal/entry + entrytypetext[]/type/entry + entry + An array that, together with literaladdress_args/literal, + can be used by the C-language function getObjectAddress() to + recreate the object address in a remote server containing a similar object. + /entry +/row +row + entryliteraladdress_args/literal/entry + entrytypetext[]/type/entry + entry + See literaladdress_names/literal above. + /entry +/row I couldn't find a function called getObjectAddress anywhere. Typo? Ah, yeah, it's get_object_address actually. Also, is providing a C-language function the best we can do? The rest of the information returned by pg_event_trigger_dropped_objects is usable from any language. Well, the return value from get_object_address is an ObjectAddress. It's simple enough to create an SQL wrapper that takes the address_names/address_args arrays and return an ObjectAddress; would this be useful? An ObjectAddress consists of a classid, objid, and objsubid. pg_event_trigger_dropped_objects already returns all of those as separate fields. What am I missing? - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Heikki Linnakangas wrote: On 10/03/2014 09:06 PM, Alvaro Herrera wrote: Well, the return value from get_object_address is an ObjectAddress. It's simple enough to create an SQL wrapper that takes the address_names/address_args arrays and return an ObjectAddress; would this be useful? An ObjectAddress consists of a classid, objid, and objsubid. pg_event_trigger_dropped_objects already returns all of those as separate fields. What am I missing? Precisely the point is not returning those values, because they are useless to identify the equivalent object in a remote database. What we need is the object names and other stuff used to uniquely identify it by user-visible name. We transmit those name arrays to a remote server, then on the remote server we can run get_object_address and get the ObjectAddress, which has the classid,objid,objsubid values you cite ... but for the remote server. The object can then be dropped there. Initially we thought that we would use the object_identity object for this (which is why we invented that functionality and added the column in 9.3), but this turned out not to work very well for unusual object types; hence this patch. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
Alvaro Herrera wrote: Precisely the point is not returning those values, because they are useless to identify the equivalent object in a remote database. What we need is the object names and other stuff used to uniquely identify it by user-visible name. We transmit those name arrays to a remote server, then on the remote server we can run get_object_address and get the ObjectAddress, which has the classid,objid,objsubid values you cite ... but for the remote server. The object can then be dropped there. Initially we thought that we would use the object_identity object for this (which is why we invented that functionality and added the column in 9.3), but this turned out not to work very well for unusual object types; hence this patch. The other thing to keep in mind is that with all those ObjectAddress thingies you got, you cannot simply construct a DROP obj command: 1. The objects in the list might be of different types; say a table and a view that are dropped by the same command because of CASCADE. (You could just pass the CASCADE to the other side and hope that it happens to do the same thing; but if the schemas are slightly different, it might not.) 2. DROP OWNED or other commands might have dropped several objects, again of varying types. So what we do in BDR is stuff all those ObjectAddress in an array of them, and then call performMultipleDeletions. There is no way to get this functionality in non-C code; so it's hard to see that it's very useful to have a non-C way to use the original objnames/objargs arrays. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
Right. In the add to objname cases, there is already some other
routine that initialized it previously by filling in some stuff; in the
case above, this happens in the getRelationIdentity() immediately
preceding this.
In the other cases we initialize on that spot.
ahh, ok, that makes a bit more sense, sorry for missing it. Still makes
me wonder why objargs gets special treatment at the top of the function
and objnames doesn't- seems like both should be initialized either
before being passed in (and perhaps an Assert to verify that they are),
or they should both be initialized, but I tend to prefer just Assert'ing
that they are correct on entry- either both are valid pointers to empty
lists, or both NULL.
I'm also not a huge fan of the big object_type_map, but I also don't
have a better solution.
Agreed. We have the ObjectProperty array also in the same file; it
kinda looks like there is duplication here, but actually there isn't.
Yeah, I did notice that, and noticed that it's not duplication.
This whole issue is just fallout from the fact that we have three
different ways to identify object classes: the ObjectClass enum, the
ObjectType enum, and the relation OIDs of each catalog (actually a
fourth one, see below). I don't see any other nice way around this; I
guess we could try to auto-generate these tables somehow from a master
text file, or something. Not material for this patch, I think.
Agreed that this patch doesn't need to address it and not sure that a
master text file would actually be an improvement.. I had been thinking
if there was some way to have a single mapping which could be used in
either direction, but I didn't see any sensible way to make that work
given that it's not *quite* the same backwards and forewards.
Note my DDL deparse patch adds a comment:
+/* XXX merge this with ObjectTypeMap? */
static event_trigger_support_data event_trigger_support[] = {
Yes, I saw that, and that you added a comment that the new map needs to
be updated when changes are made, which is also good.
and a late revision to that patch added a new function in
event_triggers.c (not yet posted I think) due to GRANT having its own
enum of object types, AclObjectKind.
Yeah. Perhaps one day we'll unify all of these, though I'm not 100%
sure it'd be possible...
Thanks!
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] replicating DROP commands across servers
Stephen Frost wrote: * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Right. In the add to objname cases, there is already some other routine that initialized it previously by filling in some stuff; in the case above, this happens in the getRelationIdentity() immediately preceding this. In the other cases we initialize on that spot. ahh, ok, that makes a bit more sense, sorry for missing it. Still makes me wonder why objargs gets special treatment at the top of the function and objnames doesn't- seems like both should be initialized either before being passed in (and perhaps an Assert to verify that they are), or they should both be initialized, but I tend to prefer just Assert'ing that they are correct on entry- either both are valid pointers to empty lists, or both NULL. I guess I could initialize objnames to NIL also. I initialize objargs because that one is unused for a lot of object types (so I would have to set it to NIL in cases where it's not used), whereas objnames is always used and thus we know it's always initialized later. Maybe what I need here is just a longer comment explaining this ... -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Stephen Frost wrote: ahh, ok, that makes a bit more sense, sorry for missing it. Still makes me wonder why objargs gets special treatment at the top of the function and objnames doesn't- seems like both should be initialized either before being passed in (and perhaps an Assert to verify that they are), or they should both be initialized, but I tend to prefer just Assert'ing that they are correct on entry- either both are valid pointers to empty lists, or both NULL. I guess I could initialize objnames to NIL also. I initialize objargs because that one is unused for a lot of object types (so I would have to set it to NIL in cases where it's not used), whereas objnames is always used and thus we know it's always initialized later. Maybe what I need here is just a longer comment explaining this ... A one-line comment that it's always reset below would be sufficient for me. Thanks for explaining it :), Stephen signature.asc Description: Digital signature
Re: [HACKERS] replicating DROP commands across servers
On Fri, Oct 3, 2014 at 2:33 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Heikki Linnakangas wrote: On 10/03/2014 09:06 PM, Alvaro Herrera wrote: Well, the return value from get_object_address is an ObjectAddress. It's simple enough to create an SQL wrapper that takes the address_names/address_args arrays and return an ObjectAddress; would this be useful? An ObjectAddress consists of a classid, objid, and objsubid. pg_event_trigger_dropped_objects already returns all of those as separate fields. What am I missing? Precisely the point is not returning those values, because they are useless to identify the equivalent object in a remote database. What we need is the object names and other stuff used to uniquely identify it by user-visible name. We transmit those name arrays to a remote server, then on the remote server we can run get_object_address and get the ObjectAddress, which has the classid,objid,objsubid values you cite ... but for the remote server. The object can then be dropped there. Initially we thought that we would use the object_identity object for this (which is why we invented that functionality and added the column in 9.3), but this turned out not to work very well for unusual object types; hence this patch. I'm not really very convinced that it's a good idea to expose this instead of just figuring out a way to parse the object identity. But I expect to lose that argument. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Robert Haas wrote: I'm not really very convinced that it's a good idea to expose this instead of just figuring out a way to parse the object identity. That's the first thing I tried. But it's not pretty: you have to extract schema names by splitting at a period (and what if a schema name contains a period?), split out on ON for certain object types, figure out parens and argument types and names for functions and aggregates, etc. It's just not sane to try to parse such text strings. But I expect to lose that argument. Good :-) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
On 2014-10-03 14:02:09 -0300, Alvaro Herrera wrote: Since the patch has had good feedback and no further comments arise, I can just implement support for those two missing object types and push, and everybody will be happy. Right? I'd like to see a new version before that out here... I don't think there's fundamental issues, but it's complicated enough to warrant that. And I don't think it has gotten enough detailed review. I hope to be able to give a bit more of that... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
I think there's been some changes to this patch since july, care to resend a new version? Sure, here it is. The only difference with the previous version is that it now also supports column defaults. This was found to be a problem when you drop a sequence that some column default depends on -- for example a column declared SERIAL, or a sequence marked with ALTER SEQUENCE OWNED BY. The new code is able to drop both the sequence and the default value (leaving, of course, the rest of the column intact.) This required adding support for such objects in get_object_address. I have given this patch the following review: - Apply to current master (77e65bf). -- success - check-world. --success - multiple FIXME statements still exist -- are there plans to fix these items? Can the duplicated code be extracted to a static function? -Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com
Re: [HACKERS] replicating DROP commands across servers
On 2014-06-13 15:50:50 -0400, Alvaro Herrera wrote: Here's a patch implementing the proposed idea. This is used in the Bidirectional Replication stuff by Simon/Andres; it works well. I think there's been some changes to this patch since july, care to resend a new version? I think it's appropriate to mark the patch as Waiting for Author instead of Ready for Committer till then. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Andres Freund wrote:
On 2014-06-13 15:50:50 -0400, Alvaro Herrera wrote:
Here's a patch implementing the proposed idea. This is used in the
Bidirectional Replication stuff by Simon/Andres; it works well.
I think there's been some changes to this patch since july, care to
resend a new version?
Sure, here it is.
The only difference with the previous version is that it now also
supports column defaults. This was found to be a problem when you drop
a sequence that some column default depends on -- for example a column
declared SERIAL, or a sequence marked with ALTER SEQUENCE OWNED BY. The
new code is able to drop both the sequence and the default value
(leaving, of course, the rest of the column intact.) This required
adding support for such objects in get_object_address.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 17564,17569 FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
--- 17564,17582
entryObject sub-id (e.g. attribute number for columns)/entry
/row
row
+ entryliteraloriginal/literal/entry
+ entrytypebool/type/entry
+ entryFlag used to identify the root object of the deletion/entry
+/row
+row
+ entryliteralnormal/literal/entry
+ entrytypebool/type/entry
+ entry
+ Flag indicating that there's a normal dependency relationship
+ in the dependency graph leading to this object
+ /entry
+/row
+row
entryliteralobject_type/literal/entry
entrytypetext/type/entry
entryType of the object/entry
***
*** 17593,17598 FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
--- 17606,17627
identifier present in the identity is quoted if necessary.
/entry
/row
+row
+ entryliteraladdress_names/literal/entry
+ entrytypetext[]/type/entry
+ entry
+ An array that, together with literaladdress_args/literal,
+ can be used by the C-language function getObjectAddress() to
+ recreate the object address in a remote server containing a similar object.
+ /entry
+/row
+row
+ entryliteraladdress_args/literal/entry
+ entrytypetext[]/type/entry
+ entry
+ See literaladdress_names/literal above.
+ /entry
+/row
/tbody
/tgroup
/informaltable
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
***
*** 203,218 deleteObjectsInList(ObjectAddresses *targetObjects, Relation *depRel,
/*
* Keep track of objects for event triggers, if necessary.
*/
! if (trackDroppedObjectsNeeded())
{
for (i = 0; i targetObjects-numrefs; i++)
{
! ObjectAddress *thisobj = targetObjects-refs + i;
!
! if ((!(flags PERFORM_DELETION_INTERNAL))
! EventTriggerSupportsObjectClass(getObjectClass(thisobj)))
{
! EventTriggerSQLDropAddObject(thisobj);
}
}
}
--- 203,227
/*
* Keep track of objects for event triggers, if necessary.
*/
! if (trackDroppedObjectsNeeded() !(flags PERFORM_DELETION_INTERNAL))
{
for (i = 0; i targetObjects-numrefs; i++)
{
! const ObjectAddress *thisobj = targetObjects-refs[i];
! const ObjectAddressExtra *extra = targetObjects-extras[i];
! bool original = false;
! bool normal = false;
!
! if (extra-flags DEPFLAG_ORIGINAL)
! original = true;
! if (extra-flags DEPFLAG_NORMAL)
! normal = true;
! if (extra-flags DEPFLAG_REVERSE)
! normal = true;
!
! if (EventTriggerSupportsObjectClass(getObjectClass(thisobj)))
{
! EventTriggerSQLDropAddObject(thisobj, original, normal);
}
}
}
*** a/src/backend/catalog/objectaddress.c
--- b/src/backend/catalog/objectaddress.c
***
*** 417,422 static const ObjectPropertyType ObjectProperty[] =
--- 417,513
}
};
+ /*
+ * This struct maps the object types as returned by getObjectTypeDescription
+ * into ObjType enum values. Note that some enum values can be obtained by
+ * different names, and that some string object types do not have corresponding
+ * values in the enum. The user of this map must be careful to test for
+ * invalid values being returned.
+ *
+ * This follows the order of getObjectTypeDescription.
+ */
+ static const struct object_type_map
+ {
+ const char *tm_name;
+ ObjectType tm_type;
+ }
+ ObjectTypeMap[] =
+ {
+ /* OCLASS_CLASS */
+ { table, OBJECT_TABLE },
+ { index, OBJECT_INDEX },
+ { sequence, OBJECT_SEQUENCE },
+ { toast table, -1 }, /* unmapped */
+ { view, OBJECT_VIEW },
+ { materialized view, OBJECT_MATVIEW },
+ { composite type, OBJECT_COMPOSITE },
+ { foreign table, OBJECT_FOREIGN_TABLE },
+
Re: [HACKERS] replicating DROP commands across servers
Hi. I thought I'd review this patch, since pgaudit uses the pg_event_trigger_dropped_objects function. I went through the patch line by line, and I don't really have anything to say about it. I notice that there are some XXX/FIXME comments in the code, but it's not clear if those need to (or can be) fixed before the code is committed. Everything else looks good. I think this is ready for committer. -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] replicating DROP commands across servers
Here's a patch implementing the proposed idea. This is used in the
Bidirectional Replication stuff by Simon/Andres; it works well.
One thing of note is that I added output flags for normal and
original, which mostly come from performDeletion flags. This let one
select only such objects when trying to replicate a drop; otherwise,
we'd add RI triggers to the set to drop remotely, which doesn't work
because their names have OIDs embedded, and in the remote system those
are different.
One curious thing is that I had to add a hack that if an object has a
reverse flag in the ObjectAddresses array, also set the normal
output flag. (Another possibility would have been to add a reverse
output flag, but there doesn't seem to be a use for that --- it seems to
expose internals unnecessarily.)
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 17538,17543 FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
--- 17538,17556
entryObject sub-id (e.g. attribute number for columns)/entry
/row
row
+ entryliteraloriginal/literal/entry
+ entrytypebool/type/entry
+ entryFlag used to identify the root object of the deletion/entry
+/row
+row
+ entry/literalnormal/literal/entry
+ entrytypebool/type/entry
+ entry
+ Flag indicating that there's a normal dependency relationship
+ in the dependency graph leading to this object
+ /entry
+/row
+row
entryliteralobject_type/literal/entry
entrytypetext/type/entry
entryType of the object/entry
***
*** 17567,17572 FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
--- 17580,17601
identifier present in the identity is quoted if necessary.
/entry
/row
+row
+ entryliteraladdress_names/literal/entry
+ entrytypetext[]/type/entry
+ entry
+ An array that, together with literaladdress_args/literal,
+ can be used by the C-language function getObjectAddress() to
+ recreate the object address in a remote server containing a similar object.
+ entry
+/row
+row
+ entryliteraladdress_args/literal/entry
+ entrytypetext[]/type/entry
+ entry
+ See literaladdress_names/literal above.
+ /entry
+/row
/tbody
/tgroup
/informaltable
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
***
*** 203,218 deleteObjectsInList(ObjectAddresses *targetObjects, Relation *depRel,
/*
* Keep track of objects for event triggers, if necessary.
*/
! if (trackDroppedObjectsNeeded())
{
for (i = 0; i targetObjects-numrefs; i++)
{
! ObjectAddress *thisobj = targetObjects-refs + i;
!
! if ((!(flags PERFORM_DELETION_INTERNAL))
! EventTriggerSupportsObjectClass(getObjectClass(thisobj)))
{
! EventTriggerSQLDropAddObject(thisobj);
}
}
}
--- 203,227
/*
* Keep track of objects for event triggers, if necessary.
*/
! if (trackDroppedObjectsNeeded() !(flags PERFORM_DELETION_INTERNAL))
{
for (i = 0; i targetObjects-numrefs; i++)
{
! const ObjectAddress *thisobj = targetObjects-refs[i];
! const ObjectAddressExtra *extra = targetObjects-extras[i];
! bool original = false;
! bool normal = false;
!
! if (extra-flags DEPFLAG_ORIGINAL)
! original = true;
! if (extra-flags DEPFLAG_NORMAL)
! normal = true;
! if (extra-flags DEPFLAG_REVERSE)
! normal = true;
!
! if (EventTriggerSupportsObjectClass(getObjectClass(thisobj)))
{
! EventTriggerSQLDropAddObject(thisobj, original, normal);
}
}
}
*** a/src/backend/catalog/objectaddress.c
--- b/src/backend/catalog/objectaddress.c
***
*** 417,422 static const ObjectPropertyType ObjectProperty[] =
--- 417,513
}
};
+ /*
+ * This struct maps the object types as returned by getObjectTypeDescription
+ * into ObjType enum values. Note that some enum values can be obtained by
+ * different names, and that some string object types do not have corresponding
+ * values in the enum. The user of this map must be careful to test for
+ * invalid values being returned.
+ *
+ * This follows the order of getObjectTypeDescription.
+ */
+ static const struct object_type_map
+ {
+ const char *tm_name;
+ ObjectType tm_type;
+ }
+ ObjectTypeMap[] =
+ {
+ /* OCLASS_CLASS */
+ { table, OBJECT_TABLE },
+ { index, OBJECT_INDEX },
+ { sequence, OBJECT_SEQUENCE },
+ { toast table, -1 }, /* unmapped */
+ { view, OBJECT_VIEW },
+ { materialized view, OBJECT_MATVIEW },
+ { composite type, OBJECT_COMPOSITE },
+ { foreign table, OBJECT_FOREIGN_TABLE
[HACKERS] replicating DROP commands across servers
As you probably know, I've studying how to implement a system to allow replicating DDL commands to remote servers. I have a fairly good grasp of handling CREATE commands; we have another thread discussing that involving a JSON output for such commands. That hasn't been carried to completion yet; I will finish that up for 9.5. My intention is to have that code handle ALTER commands as well: I have tried some experiments and it turns out that it's a bit more complex than I would like, but it seems doable, now that I've figured out exactly how. (Of course, the ALTER bits can also be used for auditing and business rule enforcing, which are the other use-cases for this feature.) In this thread I focus on DROP. We already added support for dropped objects in 9.3 in the form of the pg_event_trigger_dropped_objects() function, which returns the set of objects dropped by any command. That's quite handy for the auditing purpose it was supposed to serve, but it's insufficient for replication because it's pretty hard to craft a command for object deletion on the remote node. So my current plan involves receiving the set of objects in the remote node, then adding them to an ObjectAddresses array, then call performMultipleDeletions() on that. As it turns out, this works quite well -- except when it doesn't. The problem is this: at the local node we have the object type and identity. We can transmit this quite fine from this node to the other node, and then in the other node parse this back into a objname list and pass that to get_object_address(); but only for objects that have a simple representation, such as schemas and relations. For procedures, constraints, triggers and other types, it's a lot of work to parse the string into the objname/objargs lists (the identities for such objects as constraint_name on schema.tablename or schema.funcname(arg1, arg2) and so on. Of course, it is *possible* to parse this, but it seems the wrong thing to do when we could just obtain the right input in the first place. My proposal therefore is to add some more columns to pg_event_trigger_dropped_objects(): more precisely, objname and objargs, which would carry exactly what get_object_address() would require to re-construct an ObjectAddress for the object being dropped at the remote end. Thoughts, objections? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] replicating DROP commands across servers
Alvaro Herrera alvhe...@2ndquadrant.com writes: My proposal therefore is to add some more columns to pg_event_trigger_dropped_objects(): more precisely, objname and objargs, which would carry exactly what get_object_address() would require to re-construct an ObjectAddress for the object being dropped at the remote end. Those aren't strings or indeed flat objects at all, but structures, so it seems like this is still rather underspecified. How will you represent something like a List of TypeName at the SQL level? 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
Re: [HACKERS] replicating DROP commands across servers
Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
My proposal therefore is to add some more columns to
pg_event_trigger_dropped_objects(): more precisely, objname and objargs,
which would carry exactly what get_object_address() would require to
re-construct an ObjectAddress for the object being dropped at the remote
end.
Those aren't strings or indeed flat objects at all, but structures, so it
seems like this is still rather underspecified. How will you represent
something like a List of TypeName at the SQL level?
Yeah, that's an ugly case. I'm thinking that I could print those like
regtype output would, and then read them back in using (something
similar to) parseTypeString(). A bit convoluted perhaps, but I think it
should work. For things such as function and cast identities, typmod
shouldn't matter AFAIK, so that loss is not significant.
Another thing this will need is a table such as
static const struct
{
const char *tm_name;
ObjectType tm_type;
}
ObjectTypeMap[] =
{
/* relation types */
{ table, OBJECT_TABLE },
{ index, OBJECT_INDEX },
{ sequence, OBJECT_SEQUENCE },
...
so that we can translate object types back into the ObjectType enum.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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
