Обсуждение: SQL access to database attributes
Hello
I am looking createdb_alterdb_grammar_refactoring.v1.patch
http://www.postgresql.org/message-id/53868E57.3030908@dalibo.com
Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility?
Surely this patch cannot be backported what is proposed there.I am looking createdb_alterdb_grammar_refactoring.v1.patch
http://www.postgresql.org/message-id/53868E57.3030908@dalibo.com
Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility?
Second question related to second patch:
Must be new syntax ALLOW_CONNECTIONS? Should not be (ENABLE | DISABLE) CONNECTION ? This doesn't need any new keyword.2014-06-21 22:11 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
PavelRegardsHelloSurely this patch cannot be backported what is proposed there.
I am looking createdb_alterdb_grammar_refactoring.v1.patch
http://www.postgresql.org/message-id/53868E57.3030908@dalibo.com
Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility?
On 06/21/2014 10:11 PM, Pavel Stehule wrote: > Hello > > I am looking createdb_alterdb_grammar_refactoring.v1.patch > > http://www.postgresql.org/message-id/53868E57.3030908@dalibo.com Thank you for looking at this. > Is any reason or is acceptable incompatible change CONNECTION_LIMIT > instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough > for breaking compatibility? How is compatibility broken? The grammar still accepts the old way, I just changed the documentation to promote the new way. > Surely this patch cannot be backported what is proposed there. There are reasons I can think of not to backport this first patch, but breaking compatibility isn't one of them. -- Vik
On 06/21/2014 10:21 PM, Pavel Stehule wrote: > Second question related to second patch: > > Must be new syntax ALLOW_CONNECTIONS? It doesn't *have* to be called that, but that's what the corresponding column in pg_database is called so why add confusion? (Actually, it's called datallowconn but that would be a silly name on the SQL level.) > Should not be (ENABLE | DISABLE) CONNECTION ? I don't think it should be, no. > This doesn't need any new keyword. None of this requires any new keywords. That's the whole point of the refactoring patch. -- Vik
2014-06-21 23:14 GMT+02:00 Vik Fearing <vik.fearing@dalibo.com>:
On 06/21/2014 10:11 PM, Pavel Stehule wrote:Thank you for looking at this.
> Hello
>
> I am looking createdb_alterdb_grammar_refactoring.v1.patch
>
> http://www.postgresql.org/message-id/53868E57.3030908@dalibo.comHow is compatibility broken? The grammar still accepts the old way, I
> Is any reason or is acceptable incompatible change CONNECTION_LIMIT
> instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough
> for breaking compatibility?
just changed the documentation to promote the new way.There are reasons I can think of not to backport this first patch, but
> Surely this patch cannot be backported what is proposed there.
breaking compatibility isn't one of them.
I am sorry, tomorrow I have to read it again
Pavel
--
Vik
Hello
I returned to review this patch after sleeping - and I have to say, these patches doesn't break a compatibility.2014-06-21 23:14 GMT+02:00 Vik Fearing <vik.fearing@dalibo.com>:
On 06/21/2014 10:11 PM, Pavel Stehule wrote:Thank you for looking at this.
> Hello
>
> I am looking createdb_alterdb_grammar_refactoring.v1.patch
>
> http://www.postgresql.org/message-id/53868E57.3030908@dalibo.comHow is compatibility broken? The grammar still accepts the old way, I
> Is any reason or is acceptable incompatible change CONNECTION_LIMIT
> instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough
> for breaking compatibility?
just changed the documentation to promote the new way.There are reasons I can think of not to backport this first patch, but
> Surely this patch cannot be backported what is proposed there.
breaking compatibility isn't one of them.
--
Vik
On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > I found only one problem - first patch introduce a new property > CONNECTION_LIMIT and replace previously used "CONNECTION LIMIT" in > documentation. But "CONNECTION LIMIT" is still supported, but it is not > documented. So for some readers it can look like breaking compatibility, but > it is false. This should be documented better. Yeah, I think the old syntax should be documented also. See, e.g., what we do for COPY. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 06/23/2014 06:21 PM, Robert Haas wrote: > On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> I found only one problem - first patch introduce a new property >> CONNECTION_LIMIT and replace previously used "CONNECTION LIMIT" in >> documentation. But "CONNECTION LIMIT" is still supported, but it is not >> documented. So for some readers it can look like breaking compatibility, but >> it is false. This should be documented better. > > Yeah, I think the old syntax should be documented also. Why do we want to document syntax that should eventually be deprecated? > See, e.g., what we do for COPY. Exactly. We're still carrying around baggage from 7.2! Backward compatibility: yes. Backward documentation: no. -- Vik
2014-06-23 18:39 GMT+02:00 Vik Fearing <vik.fearing@dalibo.com>:
On 06/23/2014 06:21 PM, Robert Haas wrote:Why do we want to document syntax that should eventually be deprecated?
> On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> I found only one problem - first patch introduce a new property
>> CONNECTION_LIMIT and replace previously used "CONNECTION LIMIT" in
>> documentation. But "CONNECTION LIMIT" is still supported, but it is not
>> documented. So for some readers it can look like breaking compatibility, but
>> it is false. This should be documented better.
>
> Yeah, I think the old syntax should be documented also.
It is fair to our users. It can be deprecated, ok, we can write in doc - this feature will be deprecated in next three years. Don't use it, but this should be documentated.
Pavel
Exactly. We're still carrying around baggage from 7.2!
> See, e.g., what we do for COPY.
Backward compatibility: yes.
Backward documentation: no.
--
Vik
On Mon, Jun 23, 2014 at 12:39 PM, Vik Fearing <vik.fearing@dalibo.com> wrote: > On 06/23/2014 06:21 PM, Robert Haas wrote: >> On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >>> I found only one problem - first patch introduce a new property >>> CONNECTION_LIMIT and replace previously used "CONNECTION LIMIT" in >>> documentation. But "CONNECTION LIMIT" is still supported, but it is not >>> documented. So for some readers it can look like breaking compatibility, but >>> it is false. This should be documented better. >> >> Yeah, I think the old syntax should be documented also. > > Why do we want to document syntax that should eventually be deprecated? Because otherwise existing users will wonder if their dumps can still be restored on newer systems. And also, because documentation is, in general, a good thing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 06/23/2014 06:45 PM, Pavel Stehule wrote: > > > > 2014-06-23 18:39 GMT+02:00 Vik Fearing <vik.fearing@dalibo.com > <mailto:vik.fearing@dalibo.com>>: > > On 06/23/2014 06:21 PM, Robert Haas wrote: > > On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule > <pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>> wrote: > >> I found only one problem - first patch introduce a new property > >> CONNECTION_LIMIT and replace previously used "CONNECTION LIMIT" in > >> documentation. But "CONNECTION LIMIT" is still supported, but it > is not > >> documented. So for some readers it can look like breaking > compatibility, but > >> it is false. This should be documented better. > > > > Yeah, I think the old syntax should be documented also. > > Why do we want to document syntax that should eventually be deprecated? > > > It is fair to our users. It can be deprecated, ok, we can write in doc - > this feature will be deprecated in next three years. Don't use it, but > this should be documentated. Okay, here is version two of the refactoring patch that documents that the with-space version is deprecated but still accepted. The feature patch is not affected by this and so I am not attaching a new version of that. -- Vik
Вложения
Vik Fearing <vik.fearing@dalibo.com> writes: > On 06/21/2014 10:11 PM, Pavel Stehule wrote: >> Is any reason or is acceptable incompatible change CONNECTION_LIMIT >> instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough >> for breaking compatibility? > How is compatibility broken? The grammar still accepts the old way, I > just changed the documentation to promote the new way. While I agree that this patch wouldn't break backwards compatibility, I don't really see what the argument is for changing the recommended spelling of the command. The difficulty with doing what you've done here is that it creates unnecessary cross-version incompatibilities; for example a 9.5 psql being used against a 9.4 server would tab-complete the wrong spelling of the option. Back-patching would change the set of versions for which the problem exists, but it wouldn't remove the problem altogether. And in fact it'd add new problems, e.g. pg_dumpall output from a 9.3.5 pg_dumpall failing to load into a 9.3.4 server. This is not the kind of change we customarily back-patch anyway. So personally I'd have just made connection_limit be an undocumented internal equivalent for CONNECTION LIMIT, and kept the latter as the preferred spelling, with no client-side changes. regards, tom lane
2014-06-29 21:09 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
+1
Vik Fearing <vik.fearing@dalibo.com> writes:
> On 06/21/2014 10:11 PM, Pavel Stehule wrote:
>> Is any reason or is acceptable incompatible change CONNECTION_LIMIT
>> instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough
>> for breaking compatibility?
> How is compatibility broken? The grammar still accepts the old way, I
> just changed the documentation to promote the new way.
While I agree that this patch wouldn't break backwards compatibility,
I don't really see what the argument is for changing the recommended
spelling of the command.
The difficulty with doing what you've done here is that it creates
unnecessary cross-version incompatibilities; for example a 9.5 psql
being used against a 9.4 server would tab-complete the wrong spelling
of the option. Back-patching would change the set of versions for
which the problem exists, but it wouldn't remove the problem altogether.
And in fact it'd add new problems, e.g. pg_dumpall output from a 9.3.5
pg_dumpall failing to load into a 9.3.4 server. This is not the kind of
change we customarily back-patch anyway.
So personally I'd have just made connection_limit be an undocumented
internal equivalent for CONNECTION LIMIT, and kept the latter as the
preferred spelling, with no client-side changes.
+1
There is no important reason do hard changes in this moment
Pavel
regards, tom lane
Vik Fearing <vik.fearing@dalibo.com> writes: > Okay, here is version two of the refactoring patch that documents that > the with-space version is deprecated but still accepted. > The feature patch is not affected by this and so I am not attaching a > new version of that. I've committed this without the changes to expose the CONNECTION_LIMIT spelling, and with some other minor fixes --- the only one of substance being that you'd broken the "foo = DEFAULT" variants of the options by removing the checks on whether defel->arg was provided. regards, tom lane