Re: SET TRANSACTION and SQL Standard

I notice that we allow commands such as

SET TRANSACTION read only read write read only;

BEGIN TRANSACTION read only read only read only;

Unsurprisingly, these violate the SQL Standard:
* p.977 section 19.1 syntax (1)
* p.957 section 17.3 syntax (2)

Not planning on fixing it myself, but others may wish to.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Simon Riggs wrote:
> I notice that we allow commands such as
>
> SET TRANSACTION read only read write read only;
>
> BEGIN TRANSACTION read only read only read only;
>
> Unsurprisingly, these violate the SQL Standard:
> * p.977 section 19.1 syntax (1)
> * p.957 section 17.3 syntax (2)

Well, we allow a lot of things. Violations of the SQL standard happen
when a command that appears in the standard doesn't do what the standard
says. Allowing commands that are not in the standard is not a violation.

While there is no huge use case for these particular cases, tolerating
redundant options is sometimes useful.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Simon Riggs wrote:
>> I notice that we allow commands such as
>> SET TRANSACTION read only read write read only;
>> BEGIN TRANSACTION read only read only read only;

> Well, we allow a lot of things. Violations of the SQL standard happen
> when a command that appears in the standard doesn't do what the standard
> says. Allowing commands that are not in the standard is not a violation.

I agree that "spec violation" is not a good argument for rejecting
these. However, self-consistency with our own common practice should
be considered. In practically every utility command we have that takes
a list of options, we throw "conflicting or redundant options" errors
in similar cases.

My own feeling is that the second example is okay but the first should
be rejected, since (a) it's quite unclear what the user wants, and (b)
the ensuing behavior would be determined by implementation artifacts
like which order we processed the options in.

regards, tom lane

On Fri, 2009-01-09 at 16:14 +0200, Peter Eisentraut wrote:
> Simon Riggs wrote:
> > I notice that we allow commands such as
> >
> > SET TRANSACTION read only read write read only;
> >
> > BEGIN TRANSACTION read only read only read only;
> >
> > Unsurprisingly, these violate the SQL Standard:
> > * p.977 section 19.1 syntax (1)
> > * p.957 section 17.3 syntax (2)
>
> Well, we allow a lot of things. Violations of the SQL standard happen
> when a command that appears in the standard doesn't do what the standard
> says. Allowing commands that are not in the standard is not a violation.

Except when the standard explicitly forbids it, as with the above.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Simon Riggs wrote:
> On Fri, 2009-01-09 at 16:14 +0200, Peter Eisentraut wrote:
>> Simon Riggs wrote:
>>> I notice that we allow commands such as
>>>
>>> SET TRANSACTION read only read write read only;
>>>
>>> BEGIN TRANSACTION read only read only read only;
>>>
>>> Unsurprisingly, these violate the SQL Standard:
>>> * p.977 section 19.1 syntax (1)
>>> * p.957 section 17.3 syntax (2)
>> Well, we allow a lot of things. Violations of the SQL standard happen
>> when a command that appears in the standard doesn't do what the standard
>> says. Allowing commands that are not in the standard is not a violation.
>
> Except when the standard explicitly forbids it, as with the above.

No, it just means that the statement "SET TRANSACTION read only read
write read only;" doesn't conform to the standard, and it's therefore
implementation-dependent what it does. See the meaning of "shall" in
Syntax Rules, section "6.3.3.2 Terms denoting rule requirements".

I agree with Tom that the 2nd form is harmless, but we should throw an
error for the first.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, 2009-01-09 at 17:11 +0200, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > On Fri, 2009-01-09 at 16:14 +0200, Peter Eisentraut wrote:
> >> Simon Riggs wrote:
> >>> I notice that we allow commands such as
> >>>
> >>> SET TRANSACTION read only read write read only;
> >>>
> >>> BEGIN TRANSACTION read only read only read only;
> >>>
> >>> Unsurprisingly, these violate the SQL Standard:
> >>> * p.977 section 19.1 syntax (1)
> >>> * p.957 section 17.3 syntax (2)
> >> Well, we allow a lot of things. Violations of the SQL standard happen
> >> when a command that appears in the standard doesn't do what the standard
> >> says. Allowing commands that are not in the standard is not a violation.
> >
> > Except when the standard explicitly forbids it, as with the above.
>
> No, it just means that the statement "SET TRANSACTION read only read
> write read only;" doesn't conform to the standard, and it's therefore
> implementation-dependent what it does. See the meaning of "shall" in
> Syntax Rules, section "6.3.3.2 Terms denoting rule requirements".

which says

"If any condition required by Syntax Rules is not satisfied when the
evaluation of Access or General Rules is attempted and the
implementation is neither processing non-conforming SQL language nor
processing conforming SQL language in a non-conforming manner, then an
exception condition is raised: syntax error or access rule violation."

If we *choose* to be an SQL implementation that conforms to the SQL
standard, then it should throw an error.

Of course, we can *choose* not to conform to the standard in this or any
case, but exactly why would we? I thought we had made a choice to
conform to the SQL Standard, unless we have specific reason not to.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> "If any condition required by Syntax Rules is not satisfied when the
> evaluation of Access or General Rules is attempted and the
> implementation is neither processing non-conforming SQL language nor
> processing conforming SQL language in a non-conforming manner, then an
> exception condition is raised: syntax error or access rule violation."

> If we *choose* to be an SQL implementation that conforms to the SQL
> standard, then it should throw an error.

That reading would forbid any nonstandard syntax whatsoever...

What this is actually describing is the "standards conformance checking"
mode that the standard says you ought to provide, but we never have
(nor have most other vendors AFAIK). In SQL92 this was described as
a "SQL Flagger" and it was optional. Not sure what the latest spec
says about that.

regards, tom lane

On Fri, 2009-01-09 at 11:20 -0500, Tom Lane wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> > "If any condition required by Syntax Rules is not satisfied when the
> > evaluation of Access or General Rules is attempted and the
> > implementation is neither processing non-conforming SQL language nor
> > processing conforming SQL language in a non-conforming manner, then an
> > exception condition is raised: syntax error or access rule violation."
>
> > If we *choose* to be an SQL implementation that conforms to the SQL
> > standard, then it should throw an error.
>
> That reading would forbid any nonstandard syntax whatsoever...

No, it does allow you to choose on a case by case basis. But yes, I had
thought our (not just my) default position was to conform to the
standard.

> What this is actually describing is the "standards conformance checking"
> mode that the standard says you ought to provide, but we never have
> (nor have most other vendors AFAIK). In SQL92 this was described as
> a "SQL Flagger" and it was optional. Not sure what the latest spec
> says about that.

I've been thinking about that as something for next release.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> Simon Riggs wrote:
>>> I notice that we allow commands such as
>>> SET TRANSACTION read only read write read only;
>>> BEGIN TRANSACTION read only read only read only;

> My own feeling is that the second example is okay but the first should
> be rejected, since (a) it's quite unclear what the user wants, and (b)
> the ensuing behavior would be determined by implementation artifacts
> like which order we processed the options in.

I think this might be best solved by providing a common function that
checks a DefElem list for duplicates. This could be used in a number of
other places as well (grep for "conflicting or redundant options").

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> I think this might be best solved by providing a common function that
> checks a DefElem list for duplicates. This could be used in a number of
> other places as well (grep for "conflicting or redundant options").

It's not clear what that would save exactly. The common coding pattern
intermixes this test with collection of the individual values for
subsequent processing, and you'd still have to do the latter. It also
seems likely that a generic check of this form would be O(N^2) replacing
code that is currently O(N). (I grant that N is usually too small for
it to matter ... but I'm not sure that's always true.)

regards, tom lane

Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > On Fri, 2009-01-09 at 16:14 +0200, Peter Eisentraut wrote:
> >> Simon Riggs wrote:
> >>> I notice that we allow commands such as
> >>>
> >>> SET TRANSACTION read only read write read only;
> >>>
> >>> BEGIN TRANSACTION read only read only read only;
> >>>
> >>> Unsurprisingly, these violate the SQL Standard:
> >>> * p.977 section 19.1 syntax (1)
> >>> * p.957 section 17.3 syntax (2)
> >> Well, we allow a lot of things. Violations of the SQL standard happen
> >> when a command that appears in the standard doesn't do what the standard
> >> says. Allowing commands that are not in the standard is not a violation.
> >
> > Except when the standard explicitly forbids it, as with the above.
>
> No, it just means that the statement "SET TRANSACTION read only read
> write read only;" doesn't conform to the standard, and it's therefore
> implementation-dependent what it does. See the meaning of "shall" in
> Syntax Rules, section "6.3.3.2 Terms denoting rule requirements".
>
> I agree with Tom that the 2nd form is harmless, but we should throw an
> error for the first.

Added to TODO:

Prevent the specification of conflicting transaction read/write
options

* http://archives.postgresql.org/pgsql-hackers/2009-01/msg00684.php

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +