Instead of maintaining an incomplete emulation of POSIX threads for
Windows, let's use an extremely minimalist macro-based abstraction for
now. A later patch will extend this, without the need to supply more
complicated pthread emulation code. (There may be a need for a more
serious portable thread abstraction in later projects, but this is not
it.)
Minor incidental problems fixed: it wasn't OK to use (pthread_t) 0 as a
special value, it wasn't OK to compare thread_t values with ==, and we
incorrectly assumed that pthread functions set errno.
Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Using pgbench in an environment with both PGPORT and PGUSER set would
have caused the generation of a debug log with an incorrect database
name due to an oversight in 412893b. Not specifying user, port and/or
database using the option switches, without their respective environment
variables, generated a log entry with empty strings, which was
rather useless.
This commit fixes this set of issues by simplifying the logic grabbing
the connection information, removing a set of getenv() calls that
emulated what libpq already does. The faulty debug log now directly
uses the information from the libpq connection, and it gets generated
after the connection to the backend is completed, not before it (in the
event of a failure libpq would complain with more information about the
connection attempt so the log is not really useful before anyway).
Author: Kota Miyake
Reviewed-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/026b3ae6fc339a18394d053c32a4463d@oss.nttdata.com
libpq's error messages for connection failures pretty well stand on
their own, especially since commits 52a10224e/27a48e5a1. Prefixing
them with 'could not connect to database "foo"' or the like is just
redundant, and perhaps even misleading if the specific database name
isn't relevant to the failure. (When it is, we trust that the
backend's error message will include the DB name.) Indeed, psql
hasn't used any such prefix in a long time. So, make all our other
programs and documentation examples agree with psql's practice.
Discussion: https://postgr.es/m/1094524.1611266589@sss.pgh.pa.us
The point of this restriction is to avoid trying to substitute variables
into timestamp literal values, which may contain strings like '12:34'.
There is a good deal more that should be done to reduce pgbench's
tendency to substitute where it shouldn't. But this is sufficient to
solve the case complained of by Jaime Soler, and it's simple enough
to back-patch.
Back-patch to v11; before commit 9d36a3866, pgbench had a slightly
different definition of what a variable name is, and anyway it seems
unwise to change long-stable branches for this.
Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2006291740420.805678@pseudo
Instead of hard-wiring specific verbosity levels into the option
processing of client applications, invent pg_logging_increase_verbosity()
and encourage clients to implement --verbose by calling that. Then,
the common convention that more -v's gets you more verbosity just works.
In particular, this allows resurrection of the debug-grade messages that
have long existed in pg_dump and its siblings. They were unreachable
before this commit due to lack of a way to select PG_LOG_DEBUG logging
level. (It appears that they may have been unreachable for some time
before common/logging.c was introduced, too, so I'm not specifically
blaming cc8d41511 for the oversight. One reason for thinking that is
that it's now apparent that _allocAH()'s message needs a null-pointer
guard. Testing might have failed to reveal that before 96bf88d52.)
Discussion: https://postgr.es/m/1173106.1600116625@sss.pgh.pa.us
This patch started out with the goal of harmonizing various arbitrary
limits on password length, but after awhile a better idea emerged:
let's just get rid of those fixed limits.
recv_password_packet() has an arbitrary limit on the packet size,
which we don't really need, so just drop it. (Note that this doesn't
really affect anything for MD5 or SCRAM password verification, since
those will hash the user's password to something shorter anyway.
It does matter for auth methods that require a cleartext password.)
Likewise remove the arbitrary error condition in pg_saslprep().
The remaining limits are mostly in client-side code that prompts
for passwords. To improve those, refactor simple_prompt() so that
it allocates its own result buffer that can be made as big as
necessary. Actually, it proves best to make a separate routine
pg_get_line() that has essentially the semantics of fgets(), except
that it allocates a suitable result buffer and hence will never
return a truncated line. (pg_get_line has a lot of potential
applications to replace randomly-sized fgets buffers elsewhere,
but I'll leave that for another patch.)
I built pg_get_line() atop stringinfo.c, which requires moving
that code to src/common/; but that seems fine since it was a poor
fit for src/port/ anyway.
This patch is mostly mine, but it owes a good deal to Nathan Bossart
who pressed for a solution to the password length problem and
created a predecessor patch. Also thanks to Peter Eisentraut and
Stephen Frost for ideas and discussion.
Discussion: https://postgr.es/m/09512C4F-8CB9-4021-B455-EF4C4F0D55A0@amazon.com
Includes some manual cleanup of places that pgindent messed up,
most of which weren't per project style anyway.
Notably, it seems some people didn't absorb the style rules of
commit c9d297751, because there were a bunch of new occurrences
of function calls with a newline just after the left paren, all
with faulty expectations about how the rest of the call would get
indented.
This option is similar to \gset, except that it is able to store all
results from combined SQL queries into separate variables. If a query
returns multiple rows, the last result is stored and if a query returns
no rows, nothing is stored.
While on it, add a TAP test for \gset to check for a failure when a
query returns multiple rows.
Author: Fabien Coelho
Reviewed-by: Ibrar Ahmed, Michael Paquier
Discussion: https://postgr.es/m/alpine.DEB.2.21.1904081914200.2529@lancre
This is similar to what pg_basebackup and pg_rewind do when reporting
cumulative data, and that's more user-friendly. Carriage returns are
now used when stderr points to a terminal, and newlines are used in
other cases, like a redirection to a log file.
Author: Amit Langote
Reviewed-by: Fabien Coelho
Discussion: https://postgr.es/m/CA+HiwqFNwEjPeVaQsp2L7DyCPv1Eg1guwhrVhzMYqUJUk8ULKg@mail.gmail.com
This patch adopts the overflow check logic introduced by commit cbdb8b4c0
into two more places. interval_mul() failed to notice if it computed a
new microseconds value that was one more than INT64_MAX, and pgbench's
double-to-int64 logic had the same sorts of edge-case problems that
cbdb8b4c0 fixed in the core code.
To make this easier to get right in future, put the guts of the checks
into new macros in c.h, and add commentary about how to use the macros
correctly.
Back-patch to all supported branches, as we did with the previous fix.
Yuya Watari
Discussion: https://postgr.es/m/CAJ2pMkbkkFw2hb9Qb1Zj8d06EhWAQXFLy73St4qWv6aX=vqnjw@mail.gmail.com
This commit allows --init-steps option in pgbench to accept "G" character
meaning server-side data generation as an initialization step.
With "G", only limited queries are sent from pgbench client and
then data is actually generated in the server. This might make
the initialization phase faster if the bandwidth between pgbench client
and the server is low.
Author: Fabien Coelho
Reviewed-by: Anna Endo, Ibrar Ahmed, Fujii Masao
Discussion: https://postgr.es/m/alpine.DEB.2.21.1904061826420.3678@lancre
These new options allow users to partition the pgbench_accounts table by
specifying the number of partitions and partitioning method. The values
allowed for partitioning method are range and hash.
This feature allows users to measure the overhead of partitioning if any.
Author: Fabien COELHO
Reviewed-by: Amit Kapila, Amit Langote, Dilip Kumar, Asif Rehman, and
Alvaro Herrera
Discussion: https://postgr.es/m/alpine.DEB.2.21.1907230826190.7008@lancre
This is still using the 2.0 version of pg_bsd_indent.
I thought it would be good to commit this separately,
so as to document the differences between 2.0 and 2.1 behavior.
Discussion: https://postgr.es/m/16296.1558103386@sss.pgh.pa.us
The original placement of this module in src/fe_utils/ is ill-considered,
because several src/common/ modules have dependencies on it, meaning that
libpgcommon and libpgfeutils now have mutual dependencies. That makes it
pointless to have distinct libraries at all. The intended design is that
libpgcommon is lower-level than libpgfeutils, so only dependencies from
the latter to the former are acceptable.
We already have the precedent that fe_memutils and a couple of other
modules in src/common/ are frontend-only, so it's not stretching anything
out of whack to treat logging.c as a frontend-only module in src/common/.
To the extent that such modules help provide a common frontend/backend
environment for the rest of common/ to use, it's a reasonable design.
(logging.c does not yet provide an ereport() emulation, but one can
dream.)
Hence, move these files over, and revert basically all of the build-system
changes made by commit cc8d41511. There are no places that need to grow
new dependencies on libpgcommon, further reinforcing the idea that this
is the right solution.
Discussion: https://postgr.es/m/a912ffff-f6e4-778a-c86a-cf5c47a12933@2ndquadrant.com
The buildfarm points out that UINT64_FORMAT might not work with sscanf;
it's calibrated for our printf implementation, which might not agree
with the platform-supplied sscanf. Fall back to just accepting an
unsigned long, which is already more than the documentation promises.
Oversight in e6c3ba7fb; back-patch to v11, as that was.
This style is frowned upon. I inadvertently introduced one in commit
fe0e0b4fc7. (My compiler does not complain about it, even though
-Wdeclaration-after-statement is specified. Weird.)
Author: Masahiko Sawada
Commit 25ee70511e introduced a memory leak in pgbench: some PGresult
structs were not being freed during error bailout, because we're now
doing more PQgetResult() calls than previously. Since there's more
cleanup code outside the discard_response() routine than in it, refactor
the cleanup code, removing the routine.
This has little effect currently, since we abandon processing after
hitting errors, but if we ever get further pgbench features (such as
testing for serializable transactions), it'll matter.
Per Coverity.
Reviewed-by: Michaël Paquier
Not required after nuking the zipfian thread-local cache.
Also add a comment about hazardous pointer punning in threadRun(),
and avoid using "thread" to refer to the threads array as a whole.
Fabien Coelho and Tom Lane, per suggestion from Alvaro Herrera
Discussion: https://postgr.es/m/alpine.DEB.2.21.1904032126060.7997@lancre
Remove the code that supported zipfian distribution parameters less
than 1.0, as it had undocumented performance hazards, and it's not
clear that the case is useful enough to justify either fixing or
documenting those hazards.
Also, since the code path for parameter > 1.0 could perform badly
for values very close to 1.0, establish a minimum allowed value
of 1.001. This solution seems superior to the previous vague
documentation warning about small values not performing well.
Fabien Coelho, per a gripe from Tomas Vondra
Discussion: https://postgr.es/m/b5e172e9-ad22-48a3-86a3-589afa20e8f7@2ndquadrant.com
This unifies the various ad hoc logging (message printing, error
printing) systems used throughout the command-line programs.
Features:
- Program name is automatically prefixed.
- Message string does not end with newline. This removes a common
source of inconsistencies and omissions.
- Additionally, a final newline is automatically stripped, simplifying
use of PQerrorMessage() etc., another common source of mistakes.
- I converted error message strings to use %m where possible.
- As a result of the above several points, more translatable message
strings can be shared between different components and between
frontends and backend, without gratuitous punctuation or whitespace
differences.
- There is support for setting a "log level". This is not meant to be
user-facing, but can be used internally to implement debug or
verbose modes.
- Lazy argument evaluation, so no significant overhead if logging at
some level is disabled.
- Some color in the messages, similar to gcc and clang. Set
PG_COLOR=auto to try it out. Some colors are predefined, but can be
customized by setting PG_COLORS.
- Common files (common/, fe_utils/, etc.) can handle logging much more
simply by just using one API without worrying too much about the
context of the calling program, requiring callbacks, or having to
pass "progname" around everywhere.
- Some programs called setvbuf() to make sure that stderr is
unbuffered, even on Windows. But not all programs did that. This
is now done centrally.
Soft goals:
- Reduces vertical space use and visual complexity of error reporting
in the source code.
- Encourages more deliberate classification of messages. For example,
in some cases it wasn't clear without analyzing the surrounding code
whether a message was meant as an error or just an info.
- Concepts and terms are vaguely aligned with popular logging
frameworks such as log4j and Python logging.
This is all just about printing stuff out. Nothing affects program
flow (e.g., fatal exits). The uses are just too varied to do that.
Some existing code had wrappers that do some kind of print-and-exit,
and I adapted those.
I tried to keep the output mostly the same, but there is a lot of
historical baggage to unwind and special cases to consider, and I
might not always have succeeded. One significant change is that
pg_rewind used to write all error messages to stdout. That is now
changed to stderr.
Reviewed-by: Donald Dong <xdong@csumb.edu>
Reviewed-by: Arthur Zakirov <a.zakirov@postgrespro.ru>
Discussion: https://www.postgresql.org/message-id/flat/6a609b43-4f57-7348-6480-bd022f924310@2ndquadrant.com
The new function is only in charge of meta commands, not SQL commands.
This change makes the code a little clearer: now all the state changes
are effected by advanceConnectionState. It also removes one indent
level, which makes the diff look bulkier than it really is.
Author: Fabien Coelho
Reviewed-by: Kirk Jamison
Discussion: https://postgr.es/m/alpine.DEB.2.21.1811240904500.12627@lancre
Partial revert of commit 6260cc550b, "pgbench: add \cset and \gset
commands".
While \gset is widely considered a useful and necessary tool for user-
defined benchmarks, \cset does not have as much value, and its
implementation was considered "not to be up to project standards"
(though I, Álvaro, can't quite understand exactly how). Therefore,
remove \cset.
Author: Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.21.1903230716030.18811@lancre
Discussion: https://postgr.es/m/201901101900.mv7zduch6sad@alvherre.pgsql
pgbench's arbitrary limit of 10 arguments for SQL statements or
metacommands is far too low. Increase it to 256.
This results in a very modest increase in memory usage, not enough to
worry about.
The maximum includes the SQL statement or metacommand. This is reflected
in the comments and revised TAP tests.
Simon Riggs and Dagfinn Ilmari Mannsåker with some light editing by me.
Reviewed by: David Rowley and Fabien Coelho
Discussion: https://postgr.es/m/CANP8+jJiMJOAf-dLoHuR-8GENiK+eHTY=Omw38Qx7j2g0NDTXA@mail.gmail.com
The pgbench regression test supposed that srandom() with a specific value
would result in deterministic output from random(), as required by POSIX.
It emerges however that OpenBSD is too smart to be constrained by mere
standards, so their random() emits nondeterministic output anyway.
While a workaround does exist, what seems like a better fix is to stop
relying on the platform's srandom()/random() altogether, so that what
you get from --random-seed=N is not merely deterministic but platform
independent. Hence, use a separate pg_jrand48() random sequence in
place of random().
Also adjust the regression test case that's supposed to detect
nondeterminism so that it's more likely to detect it; the original
choice of random_zipfian parameter tended to produce the same output
all the time even if the underlying behavior wasn't deterministic.
In passing, improve pgbench's docs about random_zipfian().
Back-patch to v11 where this code was introduced.
Fabien Coelho and Tom Lane
Discussion: https://postgr.es/m/4615.1547792324@sss.pgh.pa.us
Commit c0d0e54084 replaced the ones in the documentation, but missed out
on the ones in the code. Replace those as well, but unlike c0d0e54084,
don't backpatch the code changes to avoid breaking translations.
