On Fri, Jul 17, 2020 at 05:28:51PM +0530, vignesh C wrote: > On Fri, Jul 17, 2020 at 11:15 AM Michael Paquier <mich...@paquier.xyz> wrote: >> Not completely actually. The page of psql for \copy does not mention >> the optional where clause, and I think that it would be better to add >> that for consistency (perhaps that's the point raised by Ahsan?). I >> don't see much point in splitting the description of the meta-command >> into two lines as we already mix stdin and stdout for example which >> only apply to respectively "FROM" and "TO", so let's just append the >> conditional where clause at its end. Attached is a patch doing so >> that I intend to back-patch down to v12. > > I would like to split into 2 lines similar to documentation of > sql-copy which gives better readability, attaching a new patch in > similar lines.
Fine by me. I have applied and back-patched this part down to 12. >> Coming back to your proposal, another thing is that with your patch >> you recommend a syntax still present for compatibility reasons, but I >> don't think that we should recommend it to the users anymore, giving >> priority to the new grammar of the post-9.0 era. I would actually go >> as far as removing BINARY from the completion when specified just >> after COPY to simplify the code, and specify the list of available >> options after typing "COPY ... WITH (FORMAT ", with "text", "csv" and >> "binary". Adding completion for WHERE after COPY FROM is of course a >> good idea. > > I agree with your comments, and have made a new patch accordingly. > Thoughts? Nope, that's not what I meant. My point was to drop completely from the completion the past grammar we are keeping around for compatibility reasons, and just complete with the new grammar documented at the top of the COPY page. This leads me to the attached, which actually simplifies the code, adds more completion patterns with the mixes of WHERE/WITH depending on if FROM or TO is used, and at the end is less bug-prone if the grammar gets more extended. I have also added some completion for "WITH (FORMAT" for text, csv and binary. -- Michael
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index eb018854a5..d5a47be422 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2316,19 +2316,14 @@ psql_completion(const char *text, int start, int end)
else if (Matches("COPY|\\copy"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
" UNION ALL SELECT '('");
- /* If we have COPY BINARY, complete with list of tables */
- else if (Matches("COPY", "BINARY"))
- COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
- /* If we have COPY (, complete it with legal commands */
+ /* Complete COPY ( with legal query commands */
else if (Matches("COPY|\\copy", "("))
COMPLETE_WITH("SELECT", "TABLE", "VALUES", "INSERT", "UPDATE", "DELETE", "WITH");
- /* If we have COPY [BINARY] <sth>, complete it with "TO" or "FROM" */
- else if (Matches("COPY|\\copy", MatchAny) ||
- Matches("COPY", "BINARY", MatchAny))
+ /* Complete COPY <sth> */
+ else if (Matches("COPY|\\copy", MatchAny))
COMPLETE_WITH("FROM", "TO");
- /* If we have COPY [BINARY] <sth> FROM|TO, complete with filename */
- else if (Matches("COPY", MatchAny, "FROM|TO") ||
- Matches("COPY", "BINARY", MatchAny, "FROM|TO"))
+ /* Complete COPY <sth> FROM|TO with filename */
+ else if (Matches("COPY", MatchAny, "FROM|TO"))
{
completion_charp = "";
completion_force_quote = true; /* COPY requires quoted filename */
@@ -2340,17 +2335,28 @@ psql_completion(const char *text, int start, int end)
completion_force_quote = false;
matches = rl_completion_matches(text, complete_from_files);
}
- /* Offer options after COPY [BINARY] <sth> FROM|TO filename */
- else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny) ||
- Matches("COPY", "BINARY", MatchAny, "FROM|TO", MatchAny))
- COMPLETE_WITH("BINARY", "DELIMITER", "NULL", "CSV",
- "ENCODING");
- /* Offer options after COPY [BINARY] <sth> FROM|TO filename CSV */
- else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny, "CSV") ||
- Matches("COPY", "BINARY", MatchAny, "FROM|TO", MatchAny, "CSV"))
- COMPLETE_WITH("HEADER", "QUOTE", "ESCAPE", "FORCE QUOTE",
- "FORCE NOT NULL");
+ /* Complete COPY <smt> TO <smt> */
+ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny))
+ COMPLETE_WITH("WITH (");
+
+ /* Complete COPY <smt> FROM <smt> */
+ else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny))
+ COMPLETE_WITH("WITH (", "WHERE");
+
+ /* Complete COPY <sth> FROM|TO filename WITH ( */
+ else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny, "WITH", "("))
+ COMPLETE_WITH("FORMAT", "FREEZE", "DELIMITER", "NULL",
+ "HEADER", "QUOTE", "ESCAPE", "FORCE_QUOTE",
+ "FORCE_NOT_NULL", "FORCE_NULL", "ENCODING");
+
+ /* Complete COPY <sth> FROM|TO filename WITH (FORMAT */
+ else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny, "WITH", "(", "FORMAT"))
+ COMPLETE_WITH("binary", "csv", "text");
+
+ /* Complete COPY <smt> FROM <smt> WITH (<options>) */
+ else if (Matches("COPY|\\copy", MatchAny, "FROM", MatchAny, "WITH", MatchAny))
+ COMPLETE_WITH("WHERE");
/* CREATE ACCESS METHOD */
/* Complete "CREATE ACCESS METHOD <name>" */
signature.asc
Description: PGP signature
