Unclear error message

On Thu, Sep 20, 2018 at 09:45:09PM +0200, Laurenz Albe wrote:

That message is wrong, because "rel" and not "pkrel" is the partitioned table.
I think it should be

ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("foreign key cannot be defined on ONLY \"%s\" for a partitioned table",
Relatio
nGetRelationName(rel))));

Hmm... There are no test cases for this error message, nor for the one
below "cannot add NOT VALID foreign key to relation foo", which is a bad
idea.

The referenced relation has to be RELKIND_RELATION. Here is another
idea of error message:
cannot use ONLY on partitioned table "foo" with foreign key
--
Michael

Michael Paquier wrote:

On Thu, Sep 20, 2018 at 09:45:09PM +0200, Laurenz Albe wrote:

That message is wrong, because "rel" and not "pkrel" is the partitioned table.
I think it should be

ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("foreign key cannot be defined on ONLY \"%s\" for a partitioned table",
Relatio
nGetRelationName(rel))));

Hmm... There are no test cases for this error message, nor for the one
below "cannot add NOT VALID foreign key to relation foo", which is a bad
idea.

The referenced relation has to be RELKIND_RELATION. Here is another
idea of error message:
cannot use ONLY on partitioned table "foo" with foreign key

True; how about the attached?

I think this should go in before v11.

Yours,
Laurenz Albe

On Sat, Oct 06, 2018 at 11:16:09PM +0200, Laurenz Albe wrote:

True; how about the attached?

I think this should go in before v11.

Thanks for adding regression tests for that.

- errmsg("foreign key referencing partitioned table \"%s\" must not be ONLY",
- RelationGetRelationName(pkrel))));

Here is a counter-proposal:
"cannot use ONLY for foreign key on partitioned table \"%s\" referencing
relation \"%s\""

+-- also, adding a NOT VALID foreign key should fail
+ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT VALID;
+ERROR:  cannot add NOT VALID foreign key to relation "fk_notpartitioned_pk"
+DETAIL:  This feature is not yet supported on partitioned tables.

This error should mention "fk_partitioned_fk", and not
"fk_notpartitioned_pk", no? The foreign key is added to the former, not
the latter.
--
Michael

On Sun, Oct 07, 2018 at 05:14:30PM +0900, Michael Paquier wrote:

Here is a counter-proposal:
"cannot use ONLY for foreign key on partitioned table \"%s\" referencing
relation \"%s\""

+-- also, adding a NOT VALID foreign key should fail
+ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT VALID;
+ERROR:  cannot add NOT VALID foreign key to relation "fk_notpartitioned_pk"
+DETAIL:  This feature is not yet supported on partitioned tables.

This error should mention "fk_partitioned_fk", and not
"fk_notpartitioned_pk", no? The foreign key is added to the former, not
the latter.

And after some more study, I finish with the attached. Thoughts?
--
Michael

Michael Paquier wrote:

On Sun, Oct 07, 2018 at 05:14:30PM +0900, Michael Paquier wrote:

Here is a counter-proposal:
"cannot use ONLY for foreign key on partitioned table \"%s\" referencing
relation \"%s\""

+-- also, adding a NOT VALID foreign key should fail
+ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT VALID;
+ERROR:  cannot add NOT VALID foreign key to relation "fk_notpartitioned_pk"
+DETAIL:  This feature is not yet supported on partitioned tables.

This error should mention "fk_partitioned_fk", and not
"fk_notpartitioned_pk", no? The foreign key is added to the former, not
the latter.

And after some more study, I finish with the attached. Thoughts?

I'm fine with it.

"cannot use ONLY for foreign key on partitioned table" has a funny ring
to it (after all, ONLY was used for the table, not the foreign key), but
since I could not come up with anything better, I guess there is just
no entirely satisfactory way to phrase it tersely.

Yours,
Laurenz Albe

On Mon, Oct 08, 2018 at 08:40:49AM +0200, Laurenz Albe wrote:

I'm fine with it.

Thanks, I have pushed this version and back-patched to v11.
--
Michael