Re: replicating DROP commands across servers
Re: replicating DROP commands across servers
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: replicating DROP commands across servers
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: replicating DROP commands across servers
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: replicating DROP commands across servers
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
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.GY1768@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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: replicating DROP commands across servers
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
(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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: replicating DROP commands across servers
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
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 Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: replicating DROP commands across servers
От:
David Rowley <dgrowleyml@gmail.com>
Дата:
On 24 December 2014 at 07:41, Alvaro Herrera <alvherre@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]
Kind Regards
David Rowley
Re: replicating DROP commands across servers
От:
David Rowley <dgrowley@gmail.com>
Дата:
On 25 December 2014 at 00:34, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-12-24 21:54:20 +1300, David Rowley wrote:
> On 24 December 2014 at 07:41, Alvaro Herrera <alvherre@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
Re: replicating DROP commands across servers
От:
David Rowley <dgrowley@gmail.com>
Дата:
On 25 December 2014 at 04:47, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowley@gmail.com> writes:
> On 25 December 2014 at 00:34, Andres Freund <andres@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: replicating DROP commands across servers
От:
"Brightwell, Adam" <adam.brightwell@crunchydatasolutions.com>
Дата:
> 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.brightwell@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
Re: replicating DROP commands across servers
От:
Jeff Janes <jeff.janes@gmail.com>
Дата:
On Mon, Dec 29, 2014 at 2:15 PM, Alvaro Herrera <alvherre@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
Re: replicating DROP commands across servers
От:
Jeff Janes <jeff.janes@gmail.com>
Дата:
On Fri, Jan 2, 2015 at 9:59 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Mon, Dec 29, 2014 at 2:15 PM, Alvaro Herrera <alvherre@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 jjanesin 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 0x00000000005135ff in NameListToString (names=0x257fcf8) at namespace.c:2935
2935 if (IsA(name, String))
(gdb) bt
#0 0x00000000005135ff in NameListToString (names=0x257fcf8) at namespace.c:2935
#1 0x0000000000512f33 in DeconstructQualifiedName (names=0x257fcf8, nspname_p=0x7fff419bc960, objname_p=0x7fff419bc958) at namespace.c:2648
#2 0x000000000058a746 in LookupTypeName (pstate=0x0, typeName=0x257fd10, typmod_p=0x0, missing_ok=0 '\000') at parse_type.c:153
#3 0x00000000005220b4 in get_object_address_type (objtype=OBJECT_TYPE, objname=0x257fd50, missing_ok=0 '\000') at objectaddress.c:1306
#4 0x0000000000520cf5 in get_object_address (objtype=OBJECT_TYPE, objname=0x257fd50, objargs=0x0, relp=0x7fff419bcb58, lockmode=4, missing_ok=0 '\000')
at objectaddress.c:678
#5 0x00000000005c0f36 in ExecAlterExtensionContentsStmt (stmt=0x257fd80) at extension.c:2906
#6 0x000000000077508c 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 0x000000000077490e 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 0x0000000000773bcc 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 0x0000000000772dd6 in PortalRunUtility (portal=0x2505b90, utilityStmt=0x257fd80, isTopLevel=0 '\000', dest=0x2581b60, completionTag=0x7fff419bd100 "")
at pquery.c:1187
#10 0x0000000000772f8c in PortalRunMulti (portal=0x2505b90, isTopLevel=0 '\000', dest=0x2581b60, altdest=0x2581b60, completionTag=0x7fff419bd100 "")
at pquery.c:1318
#11 0x0000000000772560 in PortalRun (portal=0x2505b90, count=9223372036854775807, isTopLevel=0 '\000', dest=0x2581b60, altdest=0x2581b60,
completionTag=0x7fff419bd100 "") at pquery.c:816
#12 0x000000000076c868 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 0x00000000007708a2 in PostgresMain (argc=1, argv=0x24ed5e0, dbname=0x24ed4b8 "jjanes", username=0x24ed4a0 "jjanes") at postgres.c:4012
#14 0x0000000000701940 in BackendRun (port=0x250c1b0) at postmaster.c:4130
#15 0x0000000000701083 in BackendStartup (port=0x250c1b0) at postmaster.c:3805
#16 0x00000000006fd8c5 in ServerLoop () at postmaster.c:1573
#17 0x00000000006fd013 in PostmasterMain (argc=18, argv=0x24ec480) at postmaster.c:1220
#18 0x000000000066476b in main (argc=18, argv=0x24ec480) at main.c:220
Cheers,
Jeff
Re: replicating DROP commands across servers
От:
Stephen Frost <sfrost@snowman.net>
Дата:
Alvaro,
On Wednesday, October 15, 2014, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
On Wednesday, October 15, 2014, Alvaro Herrera <alvherre@2ndquadrant.com> 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.
The OBJECT_POLICY bit is on me to clean up and I'm planning to do so shortly. I agree that we likely want policies for other objects also as a couple people have brought up that idea now and will investigate it.
I'm planning to address the copy.c comments first and should have a patch later tonight.
Thanks!
Stephen
