From: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PoC: Make it possible to disallow WHERE-less UPDATE and DELETE |
Date: | 2016-09-30 03:23:13 |
Message-ID: | CAEepm=3PembgvysTv8yfjugbUzYdi1yCtnx3pPvQS9agyxoqPg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 29, 2016 at 6:19 PM, David Fetter <david(at)fetter(dot)org> wrote:
> On Thu, Sep 29, 2016 at 11:12:11AM +1300, Thomas Munro wrote:
>> On Mon, Sep 26, 2016 at 5:11 PM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> > On Mon, Sep 26, 2016 at 1:18 PM, Thomas Munro
>> > <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> >>
>> >> On Mon, Sep 19, 2016 at 4:02 PM, David Fetter <david(at)fetter(dot)org> wrote:
>> >> >
>> >> > [training_wheels_004.patch]
>> >>
>> >> [review]
>>
>> Ping.
>
> Please find attached the next revision.
I'm not sold on ERRCODE_SYNTAX_ERROR. There's nothing wrong with the
syntax, since parsing succeeded. It would be cool if we could use
ERRCODE_E_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED, though I'm not sure
what error class 38 really means. Does require_where's hook function
count as an 'external routine' and on that basis is it it allowed to
raise this error? Thoughts, anyone?
I am still seeing the underscore problem when building the docs.
Please find attached fix-docs.patch which applies on top of
training_wheels_005.patch. It fixes that problem for me.
The regression test fails. The expected error messages show the old
wording, so I think you forgot to add a file. Also, should
contrib/require_where/Makefile define REGRESS = require_where? That
would allow 'make check' from inside that directory, which is
convenient and matches other extensions. Please find attached
fix-regression-test.patch which also applies on top of
training_wheels_005.patch and fixes those things for me, and also adds
a couple of extra test cases.
Robert Haas mentioned upthread that the authors section was too short,
and your follow-up v3 patch addressed that. Somehow two authors got
lost on their way to the v5 patch. Please find attached
fix-authors.patch which also applies on top of
training_wheels_005.patch to restore them.
It would be really nice to be able to set this to 'Ready for
Committer' in this CF. Do you want to post a v6 patch or are you
happy for me to ask a committer to look at v5 + these three
corrections?
--
Thomas Munro
http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
fix-regression-test.patch | application/octet-stream | 2.5 KB |
fix-docs.patch | application/octet-stream | 2.1 KB |
fix-authors.patch | application/octet-stream | 539 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Amul Sul | 2016-09-30 03:34:29 | Re: Bug in to_timestamp(). |
Previous Message | Haribabu Kommi | 2016-09-30 02:56:26 | Re: Notice lock waits |