minor patch submission: CREATE CAST ... AS EXPLICIT

Fabien COELHO <coelho@cri.ensmp.fr> writes:

Description:
Add "AS EXPLICIT" to "CREATE CAST"
This gives a name to the default case of "CREATE CAST", which creates a
cast which must be explicitely invoked.

I'm not sure this is a good idea. The CREATE CAST syntax is in the SQL
standard, and this isn't it. Now I realize that we've extended that
statement already to cover some functionality that's not in the
standard, but that doesn't mean we should create unnecessarily
nonstandard syntax for cases that are in the standard. If a commercial
vendor did that, wouldn't you castigate them for trying to create vendor
lock-in?

From a language definition perspective, it is helpful to have a name for
every case instead of an implicit fallback, without any word to describe
it. See for instance "CREATE USER CREATEDB/NOCREATEDB" or "CREATE RULE ...
DO ALSO/INSTEAD" for similar occurences of naming default cases.

If we were working in a green field, I couldn't fault this logic ... but
we are not.

regards, tom lane

Hello Tom,

Add "AS EXPLICIT" to "CREATE CAST" This gives a name to the default
case of "CREATE CAST", which creates a cast which must be explicitely
invoked.

I'm not sure this is a good idea. The CREATE CAST syntax is in the SQL
standard, and this isn't it. Now I realize that we've extended that
statement already to cover some functionality that's not in the
standard, but that doesn't mean we should create unnecessarily
nonstandard syntax for cases that are in the standard.

The standard provides only one case, so "CAST" is good enough a name.

Once you start creating alternatives with distinct semantics, then it
helps to give the initial one a name as well to be able to discuss them
with something else that "the remaining case", or "when there is no
option", especially as there is something to discuss.

Note that the standard is still supported just the same, and the
documentation already underlines that "AS *" stuff is a pg extension,
nothing is really changed. Maybe the documentation could be clearer about
where the standard stops and where extensions start, even now without an
"AS EXPLICIT" clause.

If a commercial vendor did that, wouldn't you castigate them for trying
to create vendor lock-in?

I'm more concerned with explaining things to students, and its good to
have words and logic for that.

With respect to the standard, it seems good enough to me if (1) the
standard is well supported and (2) the documentation clearly says which
parts are extensions. If you really want to keep to the standard, then do
not offer any extension.

Moreover, this stuff is really minor compared to RULEs or many other
things specifics to pg, and the "lock-in" is light, you just have to
remove "AS EXPLICIT" to get away, no big deal.

Well, you decide anyway:-)

Have a nice day,

--
Fabien.

On lör, 2011-05-21 at 15:46 +0200, Fabien COELHO wrote:

Hello,

Please find attached a minor stylish patch. It compiles and the update
test cases work for me.

Description:

Add "AS EXPLICIT" to "CREATE CAST"

This gives a name to the default case of "CREATE CAST", which creates a
cast which must be explicitely invoked.

From a language definition perspective, it is helpful to have a name for

every case instead of an implicit fallback, without any word to describe
it. See for instance "CREATE USER CREATEDB/NOCREATEDB" or "CREATE RULE ...
DO ALSO/INSTEAD" for similar occurences of naming default cases.

Oddly enough, we did add the DO ALSO syntax much later, and no one
complained about that, as far as I recall.

Peter Eisentraut <peter_e@gmx.net> writes:

On lör, 2011-05-21 at 15:46 +0200, Fabien COELHO wrote:

From a language definition perspective, it is helpful to have a name for
every case instead of an implicit fallback, without any word to describe
it. See for instance "CREATE USER CREATEDB/NOCREATEDB" or "CREATE RULE ...
DO ALSO/INSTEAD" for similar occurences of naming default cases.

Oddly enough, we did add the DO ALSO syntax much later, and no one
complained about that, as far as I recall.

Sure, but CREATE RULE is entirely locally-grown syntax, so there is no
argument from standards compliance to consider there.

regards, tom lane

From a language definition perspective, it is helpful to have a name for
every case instead of an implicit fallback, without any word to describe
it. See for instance "CREATE USER CREATEDB/NOCREATEDB" or "CREATE RULE ...
DO ALSO/INSTEAD" for similar occurences of naming default cases.

Oddly enough, we did add the DO ALSO syntax much later, and no one
complained about that, as far as I recall.

I complained:-) and I submitted the patch then, AFAICR.

--
Fabien.

On 22 May 2011 07:27, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Hello Tom,

Add "AS EXPLICIT" to "CREATE CAST" This gives a name to the default case
of "CREATE CAST", which creates a cast which must be explicitely invoked.

I'm not sure this is a good idea.  The CREATE CAST syntax is in the SQL
standard, and this isn't it.  Now I realize that we've extended that
statement already to cover some functionality that's not in the
standard, but that doesn't mean we should create unnecessarily
nonstandard syntax for cases that are in the standard.

The standard provides only one case, so "CAST" is good enough a name.

Once you start creating alternatives with distinct semantics, then it helps
to give the initial one a name as well to be able to discuss them with
something else that "the remaining case", or "when there is no option",
especially as there is something to discuss.

Note that the standard is still supported just the same, and the
documentation already underlines that "AS *" stuff is a pg extension,
nothing is really changed. Maybe the documentation could be clearer about
where the standard stops and where extensions start, even now without an "AS
EXPLICIT" clause.

If a commercial vendor did that, wouldn't you castigate them for trying to
create vendor lock-in?

I'm more concerned with explaining things to students, and its good to have
words and logic for that.

With respect to the standard, it seems good enough to me if (1) the standard
is well supported and (2) the documentation clearly says which parts are
extensions. If you really want to keep to the standard, then do not offer
any extension.

Moreover, this stuff is really minor compared to RULEs or many other things
specifics to pg, and the "lock-in" is light, you just have to remove "AS
EXPLICIT" to get away, no big deal.

Hi Fabien,

I'm taking a look at this patch for the commitfest. On first reading
of the patch, it looked pretty sensible to me, but I had some trouble
applying it to HEAD:

error: patch failed: doc/src/sgml/ref/create_cast.sgml:20
error: doc/src/sgml/ref/create_cast.sgml: patch does not apply
error: patch failed: src/backend/parser/gram.y:499
error: src/backend/parser/gram.y: patch does not apply
error: patch failed: src/include/parser/kwlist.h:148
error: src/include/parser/kwlist.h: patch does not apply
error: patch failed: src/test/regress/expected/create_cast.out:27
error: src/test/regress/expected/create_cast.out: patch does not apply
error: patch failed: src/test/regress/sql/create_cast.sql:27
error: src/test/regress/sql/create_cast.sql: patch does not apply

Perhaps the patch could use a refresh?

Also, for what it's worth, I buy into the argument for adding AS
EXPLICIT. This stuff is all an extension to the SQL standard already;
it might as well have a well-rounded syntax.

Cheers,
BJ

On 18 June 2011 09:49, Brendan Jurd <direvus@gmail.com> wrote:

Hi Fabien,

I'm taking a look at this patch for the commitfest.  On first reading
of the patch, it looked pretty sensible to me, but I had some trouble
applying it to HEAD:

error: patch failed: doc/src/sgml/ref/create_cast.sgml:20
error: doc/src/sgml/ref/create_cast.sgml: patch does not apply
error: patch failed: src/backend/parser/gram.y:499
error: src/backend/parser/gram.y: patch does not apply
error: patch failed: src/include/parser/kwlist.h:148
error: src/include/parser/kwlist.h: patch does not apply
error: patch failed: src/test/regress/expected/create_cast.out:27
error: src/test/regress/expected/create_cast.out: patch does not apply
error: patch failed: src/test/regress/sql/create_cast.sql:27
error: src/test/regress/sql/create_cast.sql: patch does not apply

Perhaps the patch could use a refresh?

The author has yet to reply to the above -- we are still lacking a
patch version that applies cleanly to HEAD. I have marked this patch
'Waiting on Author'.

Cheers,
BJ

Hi,

I saw you added this 2-year old thread to the 2013-06 commitfest, but I
don't see any new activity. Huh?

On 28.05.2011 00:48, Fabien COELHO wrote:

From a language definition perspective, it is helpful to have a name for
every case instead of an implicit fallback, without any word to describe
it. See for instance "CREATE USER CREATEDB/NOCREATEDB" or "CREATE
RULE ...
DO ALSO/INSTEAD" for similar occurences of naming default cases.

Oddly enough, we did add the DO ALSO syntax much later, and no one
complained about that, as far as I recall.

I complained:-) and I submitted the patch then, AFAICR.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello,

I saw you added this 2-year old thread to the 2013-06 commitfest, but I don't
see any new activity. Huh?

What activity would you expect? I sent the patch 2 years ago on the list,
and now that I figured out that there is a "submitted patch list" open for
consideration I added the corresponding link so that it may come out of
oblivion.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Jun 16, 2013 at 12:25 PM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Hello,

I saw you added this 2-year old thread to the 2013-06 commitfest, but I

don't see any new activity. Huh?

What activity would you expect?

A patch which applies cleanly to git HEAD. This one doesn't for me,
although I'm not really sure why, I don't see any obvious conflicts.

Cheers,

Jeff

What activity would you expect?

A patch which applies cleanly to git HEAD. This one doesn't for me,
although I'm not really sure why, I don't see any obvious conflicts.

Please find attached a freshly generated patch against current master.

--
Fabien.

Le lundi 17 juin 2013 00:02:21, Fabien COELHO a écrit :

What activity would you expect?

A patch which applies cleanly to git HEAD. This one doesn't for me,
although I'm not really sure why, I don't see any obvious conflicts.

Please find attached a freshly generated patch against current master.

* Submission review:
patch is in unified format and apply on HEAD.
patch contains documentation, however I believe 'AS IMPLICIT' is a PostgreSQL
extension with special behavior and 'AS EXPLICIT' respect the standard except
that PostgreSQL adds only the expression 'AS EXPLICIT' (it is also the default
in the standard). So maybe it is possible to rephrase this piece:

@@ -411,8 +427,8 @@ CREATE CAST (bigint AS int4) WITH FUNCTION int4(bigint) AS 
ASSIGNMENT;
    <acronym>SQL</acronym> standard,
    except that SQL does not make provisions for binary-coercible
    types or extra arguments to implementation functions.
-   <literal>AS IMPLICIT</> is a <productname>PostgreSQL</productname>
-   extension, too.
+   <literal>AS IMPLICIT</> and <literal>AS EXPLICIT</> are
+   a <productname>PostgreSQL</productname> extension, too.
   </para>
  </refsect1>

After digging in the archive and the CF: original request is at
https://commitfest.postgresql.org/action/patch_view?id=563

* Usability review
** Does the patch actually implement that? yes
** Do we want that?
Back in 2012 Tom exposed arguments against it, or at least not a clear +1.
The patch add nothing but more explicit creation statement, it has gone
untouched for 2 years without interest so it is surely not something really
important for PostgreSQL users. However we already have non-standard words for
CREATE CAST, this new one is not very intrusive .

** Does it follow SQL spec, or the community-agreed behavior?
It does not follow SQL spec.

** Does it include pg_dump support (if applicable)?
Not but it is probably not interesting to add that to the pg_dump output: it
increases incompatibility with SQL spec for no gain. The result is that the
patch only allows to CREATE CAST..AS EXPLICIT without error. Then pg_dump
won't know if the CAST has been created with the default or an 'explicit
default'...

** Are there dangers?
It seems no.

* Feature test
** Does the feature work as advertised? Yes
** Are there corner cases the author has failed to consider?
I think no, but my skills with the parser are limited (gram.y, ...)
** Are there any assertion failures or crashes?
no

* Performance review: not relevant.

* Coding review
Patch does not pass test:
./check_keywords.pl gram.y ../../../src/include/parser/kwlist.h

I had to update the unreserved keyword list in order to be able to build
postgresql.

** Does it follow the project coding guidelines? yes
** Are there portability issues? no (only for SQL)
** Will it work on Windows/BSD etc? yes
** Are the comments sufficient and accurate? Yes
** Does it do what it says, correctly? Yes
** Does it produce compiler warnings? don't build as is. Need patch update
** Can you make it crash? no

* Architecture review
** Is everything done in a way that fits together coherently with other
features/modules? Yes
** Are there interdependencies that can cause problems? No.

I flag it 'return with feedback', please update the patch so it builds.
Everything else is ok.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

On Sat, Jun 22, 2013 at 9:16 AM, Cédric Villemain
<cedric@2ndquadrant.com> wrote:

patch is in unified format and apply on HEAD.
patch contains documentation, however I believe 'AS IMPLICIT' is a PostgreSQL
extension with special behavior and 'AS EXPLICIT' respect the standard except
that PostgreSQL adds only the expression 'AS EXPLICIT' (it is also the default
in the standard).

I object to this patch. This patch a new keyword, EXPLICIT, for
reasons that are strictly cosmetic. Everything that you can do with
this patch can also be done without this patch. It is not a good idea
to slow down parsing of every SQL statement we have just so that
someone can write CREATE CAST .. AS EXPLICIT. Granted, the parsing
slowdown for just one keyword is probably not noticeable, but it's
cumulative with every new keyword we add. Adding them to support new
features is one thing, but adding them to support purely optional
syntax is, I think, going too far.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello Robert,

I object to this patch. This patch a new keyword, EXPLICIT, for
reasons that are strictly cosmetic. Everything that you can do with
this patch can also be done without this patch. It is not a good idea
to slow down parsing of every SQL statement we have just so that
someone can write CREATE CAST .. AS EXPLICIT. Granted, the parsing
slowdown for just one keyword is probably not noticeable, but it's
cumulative with every new keyword we add. Adding them to support new
features is one thing, but adding them to support purely optional
syntax is, I think, going too far.

I partly object to the objection:-)

I agree that it may induce a very small delay to the parser, however I
*do* think that cosmetic things are important. In order to ease
understanding, learning and memorizing a language, concepts must have
names, syntaxes, and be orthogonal and symmetric where applicable.

In this example, there are 3 kinds of casts, all 3 have a conceptual name
(explicit, implicit, assignment) but only two have a syntax, and the other
one is the absence of syntax. So you have to memorize this stupid
information (which one of the three does not have a syntax) or read the
documentation every time to remember that "explicit" is the one without a
syntax. Note also that you must state "implicit" explicitely, but
"explicit" is told implicitely, which does not really help.

The impact is also on the documentation which is not symmetric because it
is based on the syntax which is not, so it is simply a little harder to
understand.

Every year I do my class about PL/pgSQL and extensions to Pg, and every
year some students will try "as explicit" because it is logical to do so.
I think that she is right and that it should work, instead of having to
explain that "explicit" is implicit when dealing with Pg casts. Although
it is my job, I would prefer to spend time explaining more interesting
things.

From the software engineering point of view, having a syntax for all case
means that the developer must think about which kind of cast she really
wants, instead of doing the default thing just because it is the default.

So in my mind the tradeoff is between people time & annoyance and a few
machine cycles, and I have no hesitation to choose the later.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello Cï¿œdric,

So maybe it is possible to rephrase this piece:
-   <literal>AS IMPLICIT</> is a <productname>PostgreSQL</productname>
-   extension, too.
+   <literal>AS IMPLICIT</> and <literal>AS EXPLICIT</> are
+   a <productname>PostgreSQL</productname> extension, too.

Ok.

Back in 2012 Tom exposed arguments against it, or at least not a clear +1.
The patch add nothing but more explicit creation statement, it has gone
untouched for 2 years without interest so it is surely not something really
important for PostgreSQL users.

Elegant is important:-) See my answer to Robert's objection.

* Coding review
Patch does not pass test:
./check_keywords.pl gram.y ../../../src/include/parser/kwlist.h

Oops! That is not elegant!

I had to update the unreserved keyword list in order to be able to build
postgresql.

** Does it produce compiler warnings? don't build as is. Need patch update

Indeed.

I flag it 'return with feedback', please update the patch so it builds.
Everything else is ok.

Yep.

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I flag it 'return with feedback', please update the patch so it builds.
Everything else is ok.

Here it is.

--
Fabien.

Hello Fabien,

I flag it 'return with feedback', please update the patch so it builds.
Everything else is ok.

Here it is.

The patch does not apply and git also whines about trailing space.
It needs a v3...
Please note that a community-agreed behavior on this patch is not yet
acquired, you should consider that too.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

Here it is.

The patch does not apply and git also whines about trailing space.
It needs a v3...

The attachement here works for me.
Could you be more precise about the issue?

postgresql> git branch test master
postgresql> git checkout test
Switched to branch 'test'
postgresql> patch -p1 < ../as_explicit-v2.patch
patching file doc/src/sgml/ref/create_cast.sgml
patching file src/backend/parser/gram.y
patching file src/include/parser/kwlist.h
patching file src/test/regress/expected/create_cast.out
patching file src/test/regress/sql/create_cast.sql

Please note that a community-agreed behavior on this patch is not yet
acquired, you should consider that too.

Sure. I've sent my argumentation against Robert objections.

--
Fabien.

On 2013-06-22 15:10:07 -0400, Robert Haas wrote:

On Sat, Jun 22, 2013 at 9:16 AM, Cédric Villemain
<cedric@2ndquadrant.com> wrote:

patch is in unified format and apply on HEAD.
patch contains documentation, however I believe 'AS IMPLICIT' is a PostgreSQL
extension with special behavior and 'AS EXPLICIT' respect the standard except
that PostgreSQL adds only the expression 'AS EXPLICIT' (it is also the default
in the standard).

I object to this patch. This patch a new keyword, EXPLICIT, for
reasons that are strictly cosmetic. Everything that you can do with
this patch can also be done without this patch. It is not a good idea
to slow down parsing of every SQL statement we have just so that
someone can write CREATE CAST .. AS EXPLICIT. Granted, the parsing
slowdown for just one keyword is probably not noticeable, but it's
cumulative with every new keyword we add. Adding them to support new
features is one thing, but adding them to support purely optional
syntax is, I think, going too far.

What about simply not using a keyword at that location at all? Something
like the attached hack?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services