Thread: insufficient qualification of some objects in dump files
Some dump objects whose names are not unique on a schema level have insufficient details in the dump TOC. For example, a column default might have a TOC entry like this: 2153; 2604 39696 DEFAULT public a rolename Note that this only contains the schema name and the column name, but not the table name. So this makes it impossible to filter the TOC by text search in situations like this. It looks like other object types such as triggers have the same problem. I think we should amend the archive tag for these kinds of objects to include the table name, so it might look like: 2153; 2604 39696 DEFAULT public test a rolename Comments?
On Fri, Jan 29, 2016 at 1:17 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > Some dump objects whose names are not unique on a schema level have > insufficient details in the dump TOC. For example, a column default > might have a TOC entry like this: > > 2153; 2604 39696 DEFAULT public a rolename > > Note that this only contains the schema name and the column name, but > not the table name. So this makes it impossible to filter the TOC by > text search in situations like this. > > It looks like other object types such as triggers have the same problem. > > I think we should amend the archive tag for these kinds of objects to > include the table name, so it might look like: > > 2153; 2604 39696 DEFAULT public test a rolename > > Comments? +1. I noticed that this limitation is present for triggers (as you mentioned), constraints, fk constraints and RLS policies which should be completed with a table name. -- Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> On Fri, Jan 29, 2016 at 1:17 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> I think we should amend the archive tag for these kinds of objects to
>> include the table name, so it might look like:
>>
>> 2153; 2604 39696 DEFAULT public test a rolename
> +1. I noticed that this limitation is present for triggers (as you
> mentioned), constraints, fk constraints and RLS policies which should
> be completed with a table name.
How can we do this without an archive format version bump ... or were
you assuming that that would be an acceptable price? (It's not like
we haven't done those before, so maybe it is.)
regards, tom lane
On Fri, Jan 29, 2016 at 3:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Michael Paquier <michael.paquier@gmail.com> writes: > > On Fri, Jan 29, 2016 at 1:17 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > >> I think we should amend the archive tag for these kinds of objects to > >> include the table name, so it might look like: > >> > >> 2153; 2604 39696 DEFAULT public test a rolename > > > +1. I noticed that this limitation is present for triggers (as you > > mentioned), constraints, fk constraints and RLS policies which should > > be completed with a table name. > > How can we do this without an archive format version bump ... or were > you assuming that that would be an acceptable price? (It's not like > we haven't done those before, so maybe it is.) Yes, I am assuming that's worth the price, many people run similar relation schemas on the same database with different schema names. And Peter has a point that the current format can be confusing for the user. Sorry if I sounded like it was a bug that should be backpatched or something similar, I don't mean that. -- Michael
Peter Eisentraut wrote: > Some dump objects whose names are not unique on a schema level have > insufficient details in the dump TOC. For example, a column default > might have a TOC entry like this: > > 2153; 2604 39696 DEFAULT public a rolename > I think we should amend the archive tag for these kinds of objects to > include the table name, so it might look like: > > 2153; 2604 39696 DEFAULT public test a rolename > > Comments? If we're changing anything, I wonder if it makes sense to go for adding the object identity there. Perhaps that would make the whole thing be a little more regular, rather than have to count elements depending on object type. So in this case it'd be 2153; 2604 39696 DEFAULT public.test.a rolename -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 1/29/16 1:24 AM, Michael Paquier wrote: >> I think we should amend the archive tag for these kinds of objects to >> > include the table name, so it might look like: >> > >> > 2153; 2604 39696 DEFAULT public test a rolename >> > >> > Comments? > +1. I noticed that this limitation is present for triggers (as you > mentioned), constraints, fk constraints and RLS policies which should > be completed with a table name. Thank you for collecting this list. Attached is a patch for this. Tom thought this might require an archive version dump, but I'm not sure. The tags are more of an informational string for human consumption, not strictly part of the archive format.
Attachment
On Fri, Feb 26, 2016 at 9:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 1/29/16 1:24 AM, Michael Paquier wrote: >>> I think we should amend the archive tag for these kinds of objects to >>> > include the table name, so it might look like: >>> > >>> > 2153; 2604 39696 DEFAULT public test a rolename >>> > >>> > Comments? >> +1. I noticed that this limitation is present for triggers (as you >> mentioned), constraints, fk constraints and RLS policies which should >> be completed with a table name. > > Thank you for collecting this list. Attached is a patch for this. Visibly rules are on the stack as well, I forgot to mention them, and your updated patch does the job correctly. > Tom thought this might require an archive version dump, but I'm not > sure. The tags are more of an informational string for human > consumption, not strictly part of the archive format. Hm, the TOC entry, with its tag changed, is part of the dump, and this is written in the archive, but the shape of TocEntry does not change so this is really debatable. It is true that pg_restore -L is able to work even if the tag output is changed, still now that I think about that a version bump would be preferrable: what is generated by the bump is changed after all. The previous version upgrades that are K_VERS_1_11 or K_VERS_1_6 working on TOC did add new fields in the structure TocEntry and influenced pg_restore. -- Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> On Fri, Feb 26, 2016 at 9:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> Tom thought this might require an archive version dump, but I'm not
>> sure. The tags are more of an informational string for human
>> consumption, not strictly part of the archive format.
> Hm, the TOC entry, with its tag changed, is part of the dump, and this
> is written in the archive, but the shape of TocEntry does not change
> so this is really debatable.
I had in mind that we would add a separate field for tag's schema name to
TocEntry, which surely would require an archive format number bump.
As the patch is presented, I agree with Peter that it does not really
need a format number bump. The question that has to be answered is
whether this solution is good enough? You could not trust it for
automated processing of tags --- it's easy to think of cases in which the
schema/object name separation would be ambiguous. So is the tag really
"strictly for human consumption"? I'm not sure about that.
regards, tom lane
Hi Peter, On 2/26/16 1:30 AM, Tom Lane wrote: > Michael Paquier <michael.paquier@gmail.com> writes: >> On Fri, Feb 26, 2016 at 9:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote: >>> Tom thought this might require an archive version dump, but I'm not >>> sure. The tags are more of an informational string for human >>> consumption, not strictly part of the archive format. > >> Hm, the TOC entry, with its tag changed, is part of the dump, and this >> is written in the archive, but the shape of TocEntry does not change >> so this is really debatable. > > I had in mind that we would add a separate field for tag's schema name to > TocEntry, which surely would require an archive format number bump. > As the patch is presented, I agree with Peter that it does not really > need a format number bump. The question that has to be answered is > whether this solution is good enough? You could not trust it for > automated processing of tags --- it's easy to think of cases in which the > schema/object name separation would be ambiguous. So is the tag really > "strictly for human consumption"? I'm not sure about that. It looks like there is still some discussion to be had here about whether a "human readable" solution is enough. Until that's resolved I've marked this patch "Waiting on Author". -- -David david@pgmasters.net
On 2/26/16 1:30 AM, Tom Lane wrote: > As the patch is presented, I agree with Peter that it does not really > need a format number bump. The question that has to be answered is > whether this solution is good enough? You could not trust it for > automated processing of tags --- it's easy to think of cases in which the > schema/object name separation would be ambiguous. So is the tag really > "strictly for human consumption"? I'm not sure about that. Well what are those tags for? They are not used by pg_restore, so they are for users. My understanding is that the tags help in editing a TOC list for use by pg_restore. What pg_restore actually reads are the OIDs, but the tags are there so users can edit the files. The tags can also be used for ad hoc automatic processing. They are not sufficiently delimited and escaped for robustness in all cases, but it can be done if you control the inputs and know what to expect. But this is the same problem before and after my patch. Both of these cases are helped by my patch, and both of these cases were pretty broken (for the object classes in question) before my patch.
Peter Eisentraut <peter_e@gmx.net> writes:
> On 2/26/16 1:30 AM, Tom Lane wrote:
>> As the patch is presented, I agree with Peter that it does not really
>> need a format number bump. The question that has to be answered is
>> whether this solution is good enough? You could not trust it for
>> automated processing of tags --- it's easy to think of cases in which the
>> schema/object name separation would be ambiguous. So is the tag really
>> "strictly for human consumption"? I'm not sure about that.
> Well what are those tags for? They are not used by pg_restore, so they
> are for users. My understanding is that the tags help in editing a TOC
> list for use by pg_restore. What pg_restore actually reads are the
> OIDs, but the tags are there so users can edit the files. The tags can
> also be used for ad hoc automatic processing. They are not sufficiently
> delimited and escaped for robustness in all cases, but it can be done if
> you control the inputs and know what to expect. But this is the same
> problem before and after my patch.
> Both of these cases are helped by my patch, and both of these cases were
> pretty broken (for the object classes in question) before my patch.
Given the lack of any other complaints about this, I'm okay with the
approach as presented. (I haven't read the patch in detail, though.)
regards, tom lane
On Fri, Mar 18, 2016 at 5:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Given the lack of any other complaints about this, I'm okay with the > approach as presented. (I haven't read the patch in detail, though.) FWIW, I am still of the opinion that the last patch sent by Peter is in a pretty good shape. -- Michael
On Fri, Mar 18, 2016 at 2:13 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Fri, Mar 18, 2016 at 5:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Given the lack of any other complaints about this, I'm okay with the >> approach as presented. (I haven't read the patch in detail, though.) > > FWIW, I am still of the opinion that the last patch sent by Peter is > in a pretty good shape. Great. I've marked this patch as "Ready for Committer" in the CommitFest application. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Mar 18, 2016 at 11:22 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Mar 18, 2016 at 2:13 AM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> On Fri, Mar 18, 2016 at 5:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Given the lack of any other complaints about this, I'm okay with the >>> approach as presented. (I haven't read the patch in detail, though.) >> >> FWIW, I am still of the opinion that the last patch sent by Peter is >> in a pretty good shape. > > Great. I've marked this patch as "Ready for Committer" in the > CommitFest application. Peter, are you going to commit this? Feature freeze is in just a few days. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 04/05/2016 09:46 PM, Robert Haas wrote: > On Fri, Mar 18, 2016 at 11:22 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Fri, Mar 18, 2016 at 2:13 AM, Michael Paquier >> <michael.paquier@gmail.com> wrote: >>> On Fri, Mar 18, 2016 at 5:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> Given the lack of any other complaints about this, I'm okay with the >>>> approach as presented. (I haven't read the patch in detail, though.) >>> >>> FWIW, I am still of the opinion that the last patch sent by Peter is >>> in a pretty good shape. >> >> Great. I've marked this patch as "Ready for Committer" in the >> CommitFest application. > > Peter, are you going to commit this? Feature freeze is in just a few days. done