Change to appendStringInfoChar() or appendStringInfoString() where those
can be used.
Author: David Rowley <david.rowley@2ndquadrant.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
\drds leaked its second pattern argument if any, and \connect leaked
any empty-string or "-" arguments. These are old bugs, but it's hard
to imagine any real use-case where the leaks could amount to anything
meaningful, so not bothering with a back-patch.
Daniel Gustafsson and Tom Lane
Discussion: https://postgr.es/m/3641F19B-336A-431A-86CE-A80562505C5E@yesql.se
Don't move parenthesized lines to the left, even if that means they
flow past the right margin.
By default, BSD indent lines up statement continuation lines that are
within parentheses so that they start just to the right of the preceding
left parenthesis. However, traditionally, if that resulted in the
continuation line extending to the right of the desired right margin,
then indent would push it left just far enough to not overrun the margin,
if it could do so without making the continuation line start to the left of
the current statement indent. That makes for a weird mix of indentations
unless one has been completely rigid about never violating the 80-column
limit.
This behavior has been pretty universally panned by Postgres developers.
Hence, disable it with indent's new -lpl switch, so that parenthesized
lines are always lined up with the preceding left paren.
This patch is much less interesting than the first round of indent
changes, but also bulkier, so I thought it best to separate the effects.
Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org
Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
Change pg_bsd_indent to follow upstream rules for placement of comments
to the right of code, and remove pgindent hack that caused comments
following #endif to not obey the general rule.
Commit e3860ffa4d wasn't actually using
the published version of pg_bsd_indent, but a hacked-up version that
tried to minimize the amount of movement of comments to the right of
code. The situation of interest is where such a comment has to be
moved to the right of its default placement at column 33 because there's
code there. BSD indent has always moved right in units of tab stops
in such cases --- but in the previous incarnation, indent was working
in 8-space tab stops, while now it knows we use 4-space tabs. So the
net result is that in about half the cases, such comments are placed
one tab stop left of before. This is better all around: it leaves
more room on the line for comment text, and it means that in such
cases the comment uniformly starts at the next 4-space tab stop after
the code, rather than sometimes one and sometimes two tabs after.
Also, ensure that comments following #endif are indented the same
as comments following other preprocessor commands such as #else.
That inconsistency turns out to have been self-inflicted damage
from a poorly-thought-through post-indent "fixup" in pgindent.
This patch is much less interesting than the first round of indent
changes, but also bulkier, so I thought it best to separate the effects.
Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org
Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
This removes the support for building just libpq using Borland C++ or
Visual C++. This has not worked properly for years, and given the number
of complaints it's clearly not worth the maintenance burden.
Building libpq using the standard MSVC build system is of course still
supported, along with mingw.
In commit e984ef586 I (tgl) simplified the behavior of \p to just print
the current query buffer; but Daniel Vérité points out that this made it
inconsistent with the behavior of \g and \w. It should print the same
thing \g would execute. Fix that, and improve related comments.
Daniel Vérité
Discussion: https://postgr.es/m/9b4ea968-753f-4b5f-b46c-d7d3bf7c8f90@manitou-mail.org
This patch adds nestable conditional blocks to psql. The control
structure feature per se is complete, but the boolean expressions
understood by \if and \elif are pretty primitive; basically, after
variable substitution and backtick expansion, the result has to be
"true" or "false" or one of the other standard spellings of a boolean
value. But that's enough for many purposes, since you can always
do the heavy lifting on the server side; and we can extend it later.
Along the way, pay down some of the technical debt that had built up
around psql/command.c:
* Refactor exec_command() into a function per command, instead of
being a 1500-line monstrosity. This makes the file noticeably longer
because of repetitive function header/trailer overhead, but it seems
much more readable.
* Teach psql_get_variable() and psqlscanslash.l to suppress variable
substitution and backtick expansion on the basis of the conditional
stack state, thereby allowing removal of the OT_NO_EVAL kluge.
* Fix the no-doubt-once-expedient hack of sometimes silently substituting
mainloop.c's previous_buf for query_buf when calling HandleSlashCmds.
(It's a bit remarkable that commands like \r worked at all with that.)
Recall of a previous query is now done explicitly in the slash commands
where that should happen.
Corey Huinker, reviewed by Fabien Coelho, further hacking by me
Discussion: https://postgr.es/m/CADkLM=c94OSRTnat=LX0ivNq4pxDNeoomFfYvBKM5N_xfmLtAA@mail.gmail.com
Although it's reasonable to expect that most of these constants will
never change, that does not make it good programming style to hard-code
the value rather than using the RELKIND_FOO macros.
Discussion: https://postgr.es/m/11145.1488931324@sss.pgh.pa.us
It can often be useful to use expanded mode output (\x) for just a
single query. Introduce a \gx which acts exactly like \g except that it
will force expanded output mode for that one \gx call. This is simpler
than having to use \x as a toggle and also means that the user doesn't
have to worry about the current state of the expanded variable, or
resetting it later, to ensure a given query is always returned in
expanded mode.
Primairly Christoph's patch, though I did tweak the documentation and help
text a bit, and re-indented the tab completion section.
Author: Christoph Berg
Reviewed By: Daniel Verite
Discussion: https://postgr.es/m/20170127132737.6skslelaf4txs6iw%40msg.credativ.de
We had some AC_CHECK_HEADER tests that were really wastes of cycles,
because the code proceeded to #include those headers unconditionally
anyway, in all or a large majority of cases. The lack of complaints
shows that those headers are available on every platform of interest,
so we might as well let configure run a bit faster by not probing
those headers at all.
I suspect that some of the tests I left alone are equally useless, but
since all the existing #includes of the remaining headers are properly
guarded, I didn't touch them.
c.h #includes a number of core libc header files, such as <stdio.h>.
There's no point in re-including these after having read postgres.h,
postgres_fe.h, or c.h; so remove code that did so.
While at it, also fix some places that were ignoring our standard pattern
of "include postgres[_fe].h, then system header files, then other Postgres
header files". While there's not any great magic in doing it that way
rather than system headers last, it's silly to have just a few files
deviating from the general pattern. (But I didn't attempt to enforce this
globally, only in files I was touching anyway.)
I'd be the first to say that this is mostly compulsive neatnik-ism,
but over time it might save enough compile cycles to be useful.
Previously, if the user set a special variable such as ECHO to an
unrecognized value, psql would bleat but store the new value anyway, and
then fall back to a default setting for the behavior controlled by the
variable. This was agreed to be a not particularly good idea. With
this patch, invalid values result in an error message and no change in
state.
(But this applies only to variables that affect psql's behavior; purely
informational variables such as ENCODING can still be set to random
values.)
To do this, modify the API for psql's assign-hook functions so that they
can return an OK/not OK result, and give them the responsibility for
printing error messages when they reject a value. Adjust the APIs for
ParseVariableBool and ParseVariableNum to support the new behavior
conveniently.
In passing, document the variable VERSION, which had somehow escaped that.
And improve the quite-inadequate commenting in psql/variables.c.
Daniel Vérité, reviewed by Rahila Syed, some further tweaking by me
Discussion: https://postgr.es/m/7356e741-fa59-4146-a8eb-cf95fd6b21fb@mm
Where possible, use palloc or pg_malloc instead; otherwise, insert
explicit NULL checks.
Generally speaking, these are places where an actual OOM is quite
unlikely, either because they're in client programs that don't
allocate all that much, or they're very early in process startup
so that we'd likely have had a fork() failure instead. Hence,
no back-patch, even though this is nominally a bug fix.
Michael Paquier, with some adjustments by me
Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
The previous API for this function had it returning a malloc'd string.
That meant that callers had to check for NULL return, which few of them
were doing, and it also meant that callers had to remember to free()
the string later, which required extra logic in most cases.
Instead, make simple_prompt() write into a buffer supplied by the caller.
Anywhere that the maximum required input length is reasonably small,
which is almost all of the callers, we can just use a local or static
array as the buffer instead of dealing with malloc/free.
A fair number of callers used "pointer == NULL" as a proxy for "haven't
requested the password yet". Maintaining the same behavior requires
adding a separate boolean flag for that, which adds back some of the
complexity we save by removing free()s. Nonetheless, this nets out
at a small reduction in overall code size, and considerably less code
than we would have had if we'd added the missing NULL-return checks
everywhere they were needed.
In passing, clean up the API comment for simple_prompt() and get rid
of a very-unnecessary malloc/free in its Windows code path.
This is nominally a bug fix, but it does not seem worth back-patching,
because the actual risk of an OOM failure in any of these places seems
pretty tiny, and all of them are client-side not server-side anyway.
This patch is by me, but it owes a great deal to Michael Paquier
who identified the problem and drafted a patch for fixing it the
other way.
Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
This is somewhat cosmetic, since as long as you know what you are looking
at, "10.0" is a serviceable substitute for "10". But there is a potential
for confusion between version numbers with minor numbers and those without
--- we don't want people asking "why is psql saying 10.0 when my server is
10.2". Therefore, back-patch as far as practical, which turns out to be
9.3. I could have redone the patch to use fprintf(stderr) in place of
psql_error(), but it seems more work than is warranted for branches that
will be EOL or nearly so by the time v10 comes out.
Although only psql seems to contain any code that needs this, I chose
to put the support function into fe_utils, since it seems likely we'll
need it in other client programs in future. (In 9.3-9.5, use dumputils.c,
the predecessor of fe_utils/string_utils.c.)
In HEAD, also fix the backend code that whines about loadable-library
version mismatch. I don't see much need to back-patch that.
Due to simplistic quoting and confusion of database names with conninfo
strings, roles with the CREATEDB or CREATEROLE option could escalate to
superuser privileges when a superuser next ran certain maintenance
commands. The new coding rule for PQconnectdbParams() calls, documented
at conninfo_array_parse(), is to pass expand_dbname=true and wrap
literal database names in a trivial connection string. Escape
zero-length values in appendConnStrVal(). Back-patch to 9.1 (all
supported versions).
Nathan Bossart, Michael Paquier, and Noah Misch. Reviewed by Peter
Eisentraut. Reported by Nathan Bossart.
Security: CVE-2016-5424
The decision to reuse values of parameters from a previous connection
has been based on whether the new target is a conninfo string. Add this
means of overriding that default. This feature arose as one component
of a fix for security vulnerabilities in pg_dump, pg_dumpall, and
pg_upgrade, so back-patch to 9.1 (all supported versions). In 9.3 and
later, comment paragraphs that required update had already-incorrect
claims about behavior when no connection is open; fix those problems.
Security: CVE-2016-5424
This allows the timestamps to follow local conventions (in particular,
they respond to the LC_TIME environment setting). In C locale you get
the same results as before. It seems like a good idea to do this now not
later because we already changed the format of \watch headers for 9.6.
Also, increase the buffer sizes a tad to ensure there's enough space for
translated strings.
Discussion: <20160612145532.GA22965@postgresql.kr>
\crosstabview interpreted its arguments in an unusual way, including
doing case-insensitive matching of unquoted column names, which is
surely not the right thing. Rip that out in favor of doing something
equivalent to the dequoting/case-folding rules used by other psql
commands. To keep it simple, change the syntax so that the optional
sort column is specified as a separate argument, instead of the
also-quite-unusual syntax that attached it to the colH argument with
a colon.
Also, rework the error messages to be closer to project style.
\crosstabview is a completely different way to display results from a
query: instead of a vertical display of rows, the data values are placed
in a grid where the column and row headers come from the data itself,
similar to a spreadsheet.
The sort order of the horizontal header can be specified by using
another column in the query, and the vertical header determines its
ordering from the order in which they appear in the query.
This only allows displaying a single value in each cell. If more than
one value correspond to the same cell, an error is thrown. Merging of
values can be done in the query itself, if necessary. This may be
revisited in the future.
Author: Daniel Verité
Reviewed-by: Pavel Stehule, Dean Rasheed
This will prevent users from creating roles which begin with "pg_" and
will check for those roles before allowing an upgrade using pg_upgrade.
This will allow for default roles to be provided at initdb time.
Reviews by José Luis Tallón and Robert Haas
\gexec executes the just-entered query, like \g, but instead of printing
the results it takes each field as a SQL command to send to the server.
Computing a series of queries to be executed is a fairly common thing,
but up to now you always had to resort to kluges like writing the queries
to a file and then inputting the file. Now it can be done with no
intermediate step.
The implementation is fairly straightforward except for its interaction
with FETCH_COUNT. ExecQueryUsingCursor isn't capable of being called
recursively, and even if it were, its need to create a transaction
block interferes unpleasantly with the desired behavior of \gexec after
a failure of a generated query (i.e., that it can continue). Therefore,
disable use of ExecQueryUsingCursor when doing the master \gexec query.
We can still apply it to individual generated queries, however, and there
might be some value in doing so.
While testing this feature's interaction with single-step mode, I (tgl) was
led to conclude that SendQuery needs to recognize SIGINT (cancel_pressed)
as a negative response to the single-step prompt. Perhaps that's a
back-patchable bug fix, but for now I just included it here.
Corey Huinker, reviewed by Jim Nasby, Daniel Vérité, and myself
Often, upon getting an unexpected error in psql, one's first wish is that
the verbosity setting had been higher; for example, to be able to see the
schema-name field or the server code location info. Up to now the only way
has been to adjust the VERBOSITY variable and repeat the failing query.
That's a pain, and it doesn't work if the error isn't reproducible.
This commit adds a psql feature that redisplays the most recent server
error at full verbosity, without needing to make any variable changes or
re-execute the failed command. We just need to hang onto the latest error
PGresult in case the user executes \errverbose, and then apply libpq's
new PQresultVerboseErrorMessage() function to it. This will consume
some trivial amount of psql memory, but otherwise the cost when the
feature isn't used should be negligible.
Alex Shulgin, reviewed by Daniel Vérité, some improvements by me
Per discussion, we want to create a static library and put the stuff into
it that until now has been shared across src/bin/ directories by ad-hoc
methods like symlinking a source file. This commit creates the library and
populates it with a couple of files that contain the widely-useful portions
of pg_dump's dumputils.c file. dumputils.c survives, because it has some
stuff that didn't seem appropriate for fe_utils, but it's significantly
smaller and is no longer referenced from any other directory.
Follow-on patches will move more stuff into fe_utils.
The Mkvcbuild.pm hacking here is just a best guess; we'll see how the
buildfarm likes it.
Instead of just "2" seconds, allow eg. "2.5" seconds. Per request
from Alvaro Herrera. No docs change since the docs didn't say you
couldn't do this already.
Include the \pset title string if there is one, and shorten the prefab
part of the header to be "timestamp (every Ns)". Per suggestion by
David Johnston.
Michael Paquier and Tom Lane
This gets us to a point where psqlscan.l can be used by other frontend
programs for the same purpose psql uses it for, ie to detect when it's
collected a complete SQL command from input that is divided across
line boundaries. Moreover, other programs can supply their own lexers
for backslash commands of their own choosing. A follow-on patch will
use this in pgbench.
The end result here is roughly the same as in Kyotaro Horiguchi's
0001-Make-SQL-parser-part-of-psqlscan-independent-from-ps.patch, although
the details of the method for switching between lexers are quite different.
Basically, in this patch we share the entire PsqlScanState, YY_BUFFER_STATE
stack, *and* yyscan_t between different lexers. The only thing we need
to do to switch to a different lexer is to make sure the start_state is
valid for the new lexer. This works because flex doesn't keep any other
persistent state that depends on the specific lexing tables generated for
a particular .l file. (We are assuming that both lexers are built with
the same flex version, or at least versions that are compatible with
respect to the contents of yyscan_t; but that doesn't seem likely to
be a big problem in practice, considering how slowly flex changes.)
Aside from being more efficient than Horiguchi-san's original solution,
this avoids possible corner-case changes in semantics: the original code
was capable of popping the input buffer stack while still staying in
backslash-related parsing states. I'm not sure that that equates to any
useful user-visible behaviors, but I'm not sure it doesn't either, so
I'm loath to assume that we only need to consider the topmost buffer when
parsing a backslash command.
I've attempted to update the MSVC build scripts for the added .l file,
but will rely on the buildfarm to see if I missed anything.
Kyotaro Horiguchi and Tom Lane
To support this, we must reconcile some historical anomalies in the
behavior of -c. In particular, as a backward-incompatibility, -c no
longer implies --no-psqlrc.
Pavel Stehule (code) and Catalin Iacob (documentation). Review by
Michael Paquier and myself. Proposed behavior per Tom Lane.
Formerly, if "psql -o foo" failed to open the output file "foo", it would
print an error message but then carry on as though -o had not been
specified at all. This seems contrary to expectation: a program that
cannot open its output file normally fails altogether. Make psql do
exit(1) after reporting the error.
If "\o foo" failed to open "foo", it would print an error message but then
reset the output file to stdout, as if the argument had been omitted.
This is likewise pretty surprising behavior. Make it keep the previous
output state, instead.
psql keeps SIGPIPE interrupts disabled when it is writing to a pipe, either
a pipe specified by -o/\o or a transient pipe opened for purposes such as
using a pager on query output. The logic for this was too simple and could
sometimes re-enable SIGPIPE when a -o pipe was still active, thus possibly
leading to an unexpected psql crash later.
Fixing the last point required getting rid of the kluge in PrintQueryTuples
and ExecQueryUsingCursor whereby they'd transiently change the global
queryFout state, but that seems like good cleanup anyway.
Back-patch to 9.5 but not further; these are minor-enough issues that
changing the behavior in stable branches doesn't seem appropriate.
Previously, if no host information had been specified at connection time,
PQhost() would return NULL (unless you are on Windows, in which case you
got "localhost"). This is an unhelpful definition for a couple of reasons:
it can cause corner-case crashes in applications (cf commit c5ef8ce53d),
and there's no well-defined way for applications to find out the socket
directory path that's actually in use. As an example of the latter
problem, psql substituted DEFAULT_PGSOCKET_DIR for NULL in a couple of
places, but this is subtly wrong because it's conceivable that psql is
using a libpq shared library that was built with a different setting.
Hence, change PQhost() to return DEFAULT_PGSOCKET_DIR when appropriate,
and strip out the now-dead substitutions in psql. (There is still one
remaining reference to DEFAULT_PGSOCKET_DIR in psql, in prompt.c, which
I don't see a nice way to get rid of. But it only controls a prompt
abbreviation decision, so it seems noncritical.)
Also update the docs for PQhost, which had never previously mentioned
the possibility of a socket directory path being returned. In passing
fix the outright-incorrect code comment about PGconn.pgunixsocket.
PQhost() can return NULL in non-error situations, namely when a Unix-socket
connection has been selected by default. That behavior is a tad debatable
perhaps, but for the moment we should make sure that psql copes with it.
Unfortunately, do_connect() failed to: it could pass a NULL pointer to
strcmp(), resulting in crashes on most platforms. This was reported as a
security issue by ChenQin of Topsec Security Team, but the consensus of
the security list is that it's just a garden-variety bug with no security
implications.
For paranoia's sake, I made the keep_password test not trust PQuser or
PQport either, even though I believe those will never return NULL given
a valid PGconn.
Back-patch to all supported branches.
Fix some brain fade in commit a2dabf0e1dda93c8: erroneous variable names
in docs, rearrangements that made sentences less clear not more so,
undocumented and poorly-chosen-anyway API behaviors of subroutines,
bad grammar in error messages, copy-and-paste faults.
Albe Laurenz and Tom Lane
Remove the code in plpgsql that suppressed the innermost line of CONTEXT
for messages emitted by RAISE commands. That was never more than a quick
backwards-compatibility hack, and it's pretty silly in cases where the
RAISE is nested in several levels of function. What's more, it violated
our design theory that verbosity of error reports should be controlled
on the client side not the server side.
To alleviate the resulting noise increase, introduce a feature in libpq
and psql whereby the CONTEXT field of messages can be suppressed, either
always or only for non-error messages. Printing CONTEXT for errors only
is now their default behavior.
The actual code changes here are pretty small, but the effects on the
regression test outputs are widespread. I had to edit some of the
alternative expected outputs by hand; hopefully the buildfarm will soon
find anything I fat-fingered.
In passing, fix up (again) the output line counts in psql's various
help displays. Add some commentary about how to verify them.
Pavel Stehule, reviewed by Petr Jelínek, Jeevan Chalke, and others
The psql crash happened when no current connection existed. (The second
new check is optional given today's undocumented NULL argument handling
in PQhost() etc.) Back-patch to 9.0 (all supported versions).
This is the second try at this, after fcef161729 failed miserably and
had to be reverted: as it turns out, libpq cannot depend on libpgcommon
after all. Instead of shuffling code in the master branch, make that one
just like 9.4 and accept the duplication. (This was all my own mistake,
not the patch submitter's).
psql was already accepting conninfo strings as the first parameter in
\connect, but the way it worked wasn't sane; some of the other
parameters would get the previous connection's values, causing it to
connect to a completely unexpected server or, more likely, not finding
any server at all because of completely wrong combinations of
parameters.
Fix by explicitely checking for a conninfo-looking parameter in the
dbname position; if one is found, use its complete specification rather
than mix with the other arguments. Also, change tab-completion to not
try to complete conninfo/URI-looking "dbnames" and document that
conninfos are accepted as first argument.
There was a weak consensus to backpatch this, because while the behavior
of using the dbname as a conninfo is nowhere documented for \connect, it
is reasonable to expect that it works because it does work in many other
contexts. Therefore this is backpatched all the way back to 9.0.
Author: David Fetter, Andrew Dunstan. Some editorialization by me
(probably earning a Gierth's "Sloppy" badge in the process.)
Reviewers: Andrew Gierth, Erik Rijkers, Pavel Stěhule, Stephen Frost,
Robert Haas, Andrew Dunstan.
Peter Eisentraut
Tom Lane
Heikki Linnakangas
Magnus Hagander
Stephen Frost
Bruce Momjian
Noah Misch
Alvaro Herrera
Dean Rasheed
Robert Haas