Re: Patch to add support of "IF NOT EXISTS" to others "CREATE" statements

Hi,

Did some basic checks on this patch. List-wise feedback below.

- Removed unnecessary extra-lines: Yes
- Cleanly applies to Git-Head: Yes
- Documentation Updated: Yes
- Tests Updated: Yes
- All tests pass: Yes. (But see Note below)
- Does it Work (CREATE AGGREGATE): Yes
- Does it Work (CREATE OPERATOR): Yes
- Does it Work (CREATE TYPE): Yes
- Does it Work (CREATE TEXT SEARCH): Yes
- Does it Work (CREATE COLLATION): Yes

- Do we want it?: ???

- Is this a new feature: Yes
- Does it support pg_dump: Unable to test currently :(
- Does it follow coding guidelines: Yes

- Any visible issues: No
- Any corner cases missed out: Some tests are not extensive (eg. CREATE
COLLATION).
- Performance tests required: No
- Any compiler warnings: A scan.c warning (scan.c:10181:23: warning: unused
variable ‘yyg’ [-Wunused-variable]) although I doubt that is being caused
by this patch.
- Are comments sufficient: Can't comment much on code comments.

- Others:
Number of new lines added not covered by tests: ~208

======
A typical kind of ERROR is emitted in most tests. (Verified at least in
CREATE AGGREGATE / OPERATOR / TEXT SEARCH TEMPLATE).

For e.g. CREATE OPERATOR IF NOT EXISTS tries to create an OPERATOR that is
already created in the test a few lines above. So although the feature is
tested, the test unnecessarily creates the first OPERATOR. If you need to
maintain 'completeness' within each tests, you could use unique numbering
of objects instead.

CREATE OPERATOR ## (
leftarg = path,
rightarg = path,
procedure = path_inter,
commutator = ##
);
CREATE OPERATOR ## (
leftarg = path,
rightarg = path,
procedure = path_inter,
commutator = ##
);
ERROR: operator ## already exists
CREATE OPERATOR IF NOT EXISTS ## (
leftarg = path,
rightarg = path,
procedure = path_inter,
commutator = ##
);
NOTICE: operator ## already exists, skipping

=====

--
Robins Tharakan

On 24 May 2013 20:52, Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>wrote:

> Hi all,
>
> I working in a patch to include support of "IF NOT EXISTS" into "CREATE"
> statements that not have it yet.
>
> I started with "DefineStmt" section from "src/backend/parser/gram.y":
> - CREATE AGGREGATE [ IF NOT EXISTS ] ...
> - CREATE OPERATOR [ IF NOT EXISTS ] ...
> - CREATE TYPE [ IF NOT EXISTS ] ... [AS [{ENUM | RANGE}] (...)]
> - CREATE TEXT SEARCH {PARSER | DITIONARY | TEMPLATE | CONFIGURATION} [ IF
> NOT EXISTS ] ...
> - CREATE COLLATION [ IF NOT EXISTS ] ...
>
> My intention is cover anothers CREATE statements too, not just the above.
>
> If has no objection about this implementation I'll finish him and soon I
> sent the patch.
>
> Regards,
>
> --
> Fabrízio de Royes Mello
> Consultoria/Coaching PostgreSQL
> >> Blog sobre TI: http://fabriziomello.blogspot.com
> >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
> >> Twitter: http://twitter.com/fabriziomello
>

On 24 May 2013 20:52, Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>wrote:

> Hi all,
>
> I working in a patch to include support of "IF NOT EXISTS" into "CREATE"
> statements that not have it yet.
>
> I started with "DefineStmt" section from "src/backend/parser/gram.y":
> - CREATE AGGREGATE [ IF NOT EXISTS ] ...
> - CREATE OPERATOR [ IF NOT EXISTS ] ...
> - CREATE TYPE [ IF NOT EXISTS ] ... [AS [{ENUM | RANGE}] (...)]
> - CREATE TEXT SEARCH {PARSER | DITIONARY | TEMPLATE | CONFIGURATION} [ IF
> NOT EXISTS ] ...
> - CREATE COLLATION [ IF NOT EXISTS ] ...
>
> My intention is cover anothers CREATE statements too, not just the above.
>
> If has no objection about this implementation I'll finish him and soon I
> sent the patch.
>
> Regards,
>
> --
> Fabrízio de Royes Mello
> Consultoria/Coaching PostgreSQL
> >> Blog sobre TI: http://fabriziomello.blogspot.com
> >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
> >> Twitter: http://twitter.com/fabriziomello
>