On Tue, Dec 17, 2024 at 10:12 PM Robert Haas <robertmh...@gmail.com> wrote:
> On Wed, Oct 9, 2024 at 7:12 AM Nishant Sharma > <nishant.sha...@enterprisedb.com> wrote: > > I have included them in v3. v3 does not allow empty schema_name & > > table_name options along with column_name. > > I was looking at these patches today and trying to understand whether > the proposed error message is consistent with what we have done > elsewhere. > > The proposed error message was "cannot use empty value for option > \"%s\". I looked for error messages that contained the phrase "empty" > or "zero". I did not find a case exactly like this one. The closet > analogues I found were things like this: > > invalid cursor name: must not be empty > output cannot be empty string > DSM segment name cannot be empty > row path filter must not be empty string > > These messages aren't quite as consistent as one might wish, so it's a > little hard to judge here. Nonetheless, I propose that the error > message we use here should end with either "cannot" or "must not" > followed by either "be empty" or "be empty string". I think my > preferred variant would be "value for option \"%s\" must not be empty > string". But there's certainly oodles of room for bikeshedding. > > Apart from that, I see very little to complain about here. Once we > agree on the error message text I think something can be committed. > For everyone's convenience, I suggest merging the two patches into > one. I highly approve of separating patches by topic, but there's > usually no point in separating them by directory. > > -- > Robert Haas > EDB: http://www.enterprisedb.com Thanks Robert for your response and the review comments! I have addressed both your suggestions, by changing the error message and merging both the patches to one. PFA, patch set v4. Regards, Nishant.
v4-0001-Disallow-empty-Foreign-Table-column_name-schema_n.patch
Description: Binary data
