Thread: SQL access to database attributes

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

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

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

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