On Thu, Nov 3, 2016 at 12:26 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:
>
> A couple of doubts/suggestions:
> 1. Should pset.conn_was_reset be set to false, when we make a
> connection in do_connection()? Usually pset.conn_was_reset would be
> initialised with 0 since it's a global variable, so this may not be
> necessary. But as a precaution should we do this?
>
Hi,
Thank you for your comments.
I think this is not necessary since do_connection() is also called from
MainLoop where we clear the flag explicitly. I also don't see a way we
could enter do_connection() with the conn_was_reset flag set to true in the
first place.
It still makes sense to initialize it to false in startup.c, though.
2. Comment below should be updated to reflect the new change
> /* fall out of loop if lexer reached EOL */
> - if (scan_result == PSCAN_INCOMPLETE ||
> + if (pset.conn_was_reset ||
> + scan_result == PSCAN_INCOMPLETE ||
>
Check. See attached v2.
3. What happens when the connection is reset while the source is a
> file say specified by \i in an interactive shell?
In this case pset.cur_cmd_interactive is false (see mainloop.c:66) and we
don't attempt to reset a failed connection:
postgres(p)=# \i 1.sql
psql:1.sql:1: FATAL: terminating connection due to administrator command
psql:1.sql:1: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
psql:1.sql:1: connection to server was lost
$
The psql process even exits with an error code 2, which might be not that
expected. We could stop reading the file and reset connection afterwards,
but this is probably not that easy to achieve (think of nested \i calls).
I will still try to see if we can observe blocking status of a read on
stdin and if that might help in protecting from a more complex case with
pasting into terminal.
--
Alex
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
new file mode 100644
index a7789df..34a4507
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** CheckConnection(void)
*** 386,391 ****
--- 386,393 ----
}
else
psql_error("Succeeded.\n");
+
+ pset.conn_was_reset = true;
}
return OK;
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
new file mode 100644
index 37dfa4d..c049a39
*** a/src/bin/psql/mainloop.c
--- b/src/bin/psql/mainloop.c
*************** MainLoop(FILE *source)
*** 390,401 ****
break;
}
! /* fall out of loop if lexer reached EOL */
if (scan_result == PSCAN_INCOMPLETE ||
! scan_result == PSCAN_EOL)
break;
}
/* Add line to pending history if we didn't execute anything yet */
if (pset.cur_cmd_interactive && !line_saved_in_history)
pg_append_history(line, history_buf);
--- 390,404 ----
break;
}
! /* fall out of loop if lexer reached EOL or connection was reset */
if (scan_result == PSCAN_INCOMPLETE ||
! scan_result == PSCAN_EOL ||
! pset.conn_was_reset)
break;
}
+ pset.conn_was_reset = false;
+
/* Add line to pending history if we didn't execute anything yet */
if (pset.cur_cmd_interactive && !line_saved_in_history)
pg_append_history(line, history_buf);
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
new file mode 100644
index 8cfe9d2..39a4be0
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*************** typedef struct _psqlSettings
*** 102,107 ****
--- 102,110 ----
FILE *cur_cmd_source; /* describe the status of the current main
* loop */
bool cur_cmd_interactive;
+ bool conn_was_reset; /* indicates that the connection was reset
+ * during the last attempt to execute an
+ * interactive command */
int sversion; /* backend server version */
const char *progname; /* in case you renamed psql */
char *inputfile; /* file being currently processed, if any */
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
new file mode 100644
index 7ce05fb..e238de9
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*************** main(int argc, char *argv[])
*** 139,144 ****
--- 139,145 ----
pset.last_error_result = NULL;
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
+ pset.conn_was_reset = false;
/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */
pset.popt.topt.format = PRINT_ALIGNED;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
