On 2021-Jan-21, Zhihong Yu wrote: > It seems '\\gset or \\aset is not ' would correspond to the check more > closely. > > + if (my_command->argc != 1) > + syntax_error(source, lineno, my_command->first_line, > my_command->argv[0], > > It is possible that my_command->argc == 0 (where my_command->argv[0] > shouldn't be accessed) ?
No -- process_backslash_command reads the word and sets it as argv[0]. > + appendPQExpBufferStr(&conn->errorMessage, > + libpq_gettext("cannot queue commands > during COPY\n")); > + return false; > + break; > > Is the break necessary ? Similar comment for pqBatchProcessQueue(). Not necessary. I removed them. > +int > +PQexitBatchMode(PGconn *conn) > > Since either 0 or 1 is returned, maybe change the return type to bool. I was kinda tempted to do that, but in reality a lot of libpq's API is defined like that -- to return 0 (failure)/1 (success) as ints, not bools. For example see PQsendQuery(). I'm not inclined to do different for these new functions. (One curious case is PQsetvalue, which returns int, and is documented as "returns zero for failure" and then uses "return false"). > Also, the function operates on PGconn - should the function be static > (pqBatchProcessQueue is) ? I don't understand this suggestion. How would an application call it, if we make it static? Thanks -- Álvaro Herrera Valdivia, Chile "Cómo ponemos nuestros dedos en la arcilla del otro. Eso es la amistad; jugar al alfarero y ver qué formas se pueden sacar del otro" (C. Halloway en La Feria de las Tinieblas, R. Bradbury)