| Age | Commit message (Collapse) | Author |
|---|
|
Per discussion, this restriction isn't needed for any real security reason,
and it seems to confuse people more often than it helps them. It could
also result in some database states being unrestorable. So just drop it.
Back-patch to 9.0, where ALTER DEFAULT PRIVILEGES was introduced.
|
|
AllocateFile(), AllocateDir(), and some sister routines share a small array
for remembering requests, so that the files can be closed on transaction
failure. Previously that array had a fixed size, MAX_ALLOCATED_DESCS (32).
While historically that had seemed sufficient, Steve Toutant pointed out
that this meant you couldn't scan more than 32 file_fdw foreign tables in
one query, because file_fdw depends on the COPY code which uses
AllocateFile(). There are probably other cases, or will be in the future,
where this nonconfigurable limit impedes users.
We can't completely remove any such limit, at least not without a lot of
work, since each such request requires a kernel file descriptor and most
platforms limit the number we can have. (In principle we could
"virtualize" these descriptors, as fd.c already does for the main VFD pool,
but not without an additional layer of overhead and a lot of notational
impact on the calling code.) But we can at least let the array size be
configurable. Hence, change the code to allow up to max_safe_fds/2
allocated file requests. On modern platforms this should allow several
hundred concurrent file_fdw scans, or more if one increases the value of
max_files_per_process. To go much further than that, we'd need to do some
more work on the data structure, since the current code for closing
requests has potentially O(N^2) runtime; but it should still be all right
for request counts in this range.
Back-patch to 9.1 where contrib/file_fdw was introduced.
|
|
Long-standing code has called tolower() on identifier character bytes
with the high bit set. This is clearly an error and produces junk output
when the encoding is multi-byte. This patch therefore restricts this
activity to cases where there is a character with the high bit set AND
the encoding is single-byte.
There have been numerous gripes about this, most recently from Martin
Schäfer.
Backpatch to all live releases.
|
|
In 9.2, Unicode escape sequences are not analysed at all other than
to make sure that they are in the form \uXXXX. But in 9.3 many of the
new operators and functions try to turn JSON text values into text in
the server encoding, and this includes de-escaping Unicode escape
sequences. This processing had not taken into account the possibility
that this might contain a surrogate pair to designate a character
outside the BMP. That is now handled correctly.
This also enforces correct use of surrogate pairs, something that is not
done by the type's input routines. This fact is noted in the docs.
|
|
|
|
Andres Freund
|
|
The planner is aware that it mustn't push down upper-level quals into
subqueries if the quals reference subquery output columns that contain
set-returning functions or volatile functions, or are non-DISTINCT outputs
of a DISTINCT ON subquery. However, it missed making this check when
there were one or more levels of UNION or INTERSECT above the dangerous
expression. This could lead to "set-valued function called in context that
cannot accept a set" errors, as seen in bug #8213 from Eric Soroos, or to
silently wrong answers in the other cases.
To fix, refactor the checks so that we make the column-is-unsafe checks
during subquery_is_pushdown_safe(), which already has to recursively
inspect all arms of a set-operation tree. This makes
qual_is_pushdown_safe() considerably simpler, at the cost that we will
spend some cycles checking output columns that possibly aren't referenced
in any upper qual. But the cases where this code gets executed at all
are already nontrivial queries, so it's unlikely anybody will notice any
slowdown of planning.
This has been broken since commit 05f916e6add9726bf4ee046e4060c1b03c9961f2,
which makes the bug over ten years old. A bit surprising nobody noticed it
before now.
|
|
|
|
In commit 2c92edad48796119c83d7dbe6c33425d1924626d, I broke "EXPLAIN
(ANALYZE)" syntax, because I mistakenly thought that ANALYZE/ANALYSE were
only partially reserved and thus would be included in NonReservedWord;
but actually they're fully reserved so they still need to be called out
here.
A nicer solution would be to demote these words to type_func_name_keyword
status (they can't be less than that because of "VACUUM [ANALYZE] ColId").
While that works fine so far as the core grammar is concerned, it breaks
ECPG's grammar for reasons I don't have time to isolate at the moment.
So do this for the time being.
Per report from Kevin Grittner. Back-patch to 9.0, like the previous
commit.
|
|
The new message (and SQLSTATE) matches the corresponding error cases in
namespace.c.
This was thought to be a "can't happen" case when extension.c was written,
so we didn't think hard about how to report it. But it definitely can
happen in 9.2 and later, since we no longer require search_path to contain
any valid schema names. It's probably also possible in 9.1 if search_path
came from a noninteractive source. So, back-patch to all releases
containing this code.
Per report from Sean Chittenden, though this isn't exactly his patch.
|
|
Use the same gcc atomic functions as we do on newer ARM chips.
(Basically this is a copy and paste of the __arm__ code block,
but omitting the SWPB option since that definitely won't work.)
Back-patch to 9.2. The patch would work further back, but we'd also
need to update config.guess/config.sub in older branches to make them
build out-of-the-box, and there hasn't been demand for it.
Mark Salter
|
|
The array allocated by GetRunningTransactionLocks() needs to be pfree'd
when we're done with it. Otherwise we leak some memory during each
checkpoint, if wal_level = hot_standby. This manifests as memory bloat
in the checkpointer process, or in bgwriter in versions before we made
the checkpointer separate.
Reported and fixed by Naoya Anzai. Back-patch to 9.0 where the issue
was introduced.
In passing, improve comments for GetRunningTransactionLocks(), and add
an Assert that we didn't overrun the palloc'd array.
|
|
"eval q{foo}" used to complain that the error was on line 2 of the eval'd
string, because eval internally tacked on "\n;" so that the end of the
erroneous command was indeed on line 2. But as of Perl 5.18 it more
sanely says that the error is on line 1. To avoid Perl-version-dependent
regression test results, use "eval q{foo;}" instead in the two places
where this matters. Per buildfarm.
Since people might try to use newer Perl versions with older PG releases,
back-patch as far as 9.0 where these test cases were added.
|
|
This reverts commit a475c6036752c26dca538632b68fd2cc592976b7.
Erik Rijkers reported back in January 2013 that after the patch, if you do
"pg_dump -t myschema.mytable" to dump a single table, and restore that in
a database where myschema does not exist, the table is silently created in
pg_catalog instead. That is because pg_dump uses
"SET search_path=myschema, pg_catalog" to set schema the table is created
in. While allow_system_table_mods is not a very elegant solution to this,
we can't leave it as it is, so for now, revert it back to the way it was
previously.
|
|
A few more minor spelling corrections, no functional changes.
Thom Brown
|
|
Seems cleaner to get the currently-replayed TLI in the same call to
GetXLogReplayRecPtr that we get the WAL position. Make it more clear in the
comment what the code does when recovery has already ended
(RecoveryInProgress() will set ThisTimeLineID in that case). Finally, make
resetting ThisTimeLineID afterwards more explicit.
|
|
This change makes type_func_name_keywords less reserved than they were
before, by allowing them for role names, language names, EXPLAIN and COPY
options, and SET values for GUCs; which are all places where few if any
actual keywords could appear instead, so no new ambiguities are introduced.
The main driver for this change is to allow "COPY ... (FORMAT BINARY)"
to work without quoting the word "binary". That is an inconsistency that
has been complained of repeatedly over the years (at least by Pavel Golub,
Kurt Lidl, and Simon Riggs); but we hadn't thought of any non-ugly solution
until now.
Back-patch to 9.0 where the COPY (FORMAT BINARY) syntax was introduced.
|
|
Must cast char argument to unsigned to avoid doing the wrong thing
with high-bit-set characters. Oversight in commit
30b5ede7157e34e77c7914b8ecfd55aa8da6edc3.
|
|
Fix a few spelling mistakes.
Per bug report #8193 from Lajos Veres.
|
|
Make slightly better decisions about indentation than what pgindent
is capable of. Mostly breaking out long function calls into one
line per argument, with a few other minor adjustments.
No functional changes- all whitespace.
pgindent ran cleanly (didn't change anything) after.
Passes all regressions.
|
|
Dean Rasheed
|
|
|
|
|
|
|
|
|
|
This is the first run of the Perl-based pgindent script. Also update
pgindent instructions.
|
|
Pavan Deolasee
|
|
When COPY uses the multi-insert method to insert a batch of tuples into the
heap at a time, incorrect line number was printed if something went wrong in
inserting the index tuples (primary key failure, for exampl), or processing
after row triggers.
Fixes bug #8173 reported by Lloyd Albin. Backpatch to 9.2, where the multi-
insert code was added.
|
|
Not necessary for correctness, just to make
log_checkpoints output look less singular.
Requested by Fujii Masao
|
|
checkpointer needs to reset ThisTimeLineID after
a restartpoint to allow installing/recycling new
WAL files. If recovery has already ended this
would leave ThisTimeLineID set incorrectly and
so we must reset it otherwise later checkpoints
do not have the correct timeline.
Bug report by Heikki Linnakangas.
Further investigation by Heikki and myself.
|
|
In the primary_conninfo line that "pg_basebackup -R" generates, single
quotes in parameter values need to be escaped into \\'; the libpq parser
requires the quotes to be escaped into \', and recovery.conf parser requires
the \ to be escaped into \\.
Also, don't quote parameter values unnecessarily, to make the connection
string prettier. Most options in a libpq connection string don't need
quoting.
Reported by Hari Babu, closer analysis by Zoltan Boszormenyi, although I
didn't use his patch.
|
|
Clarify that this option doesn't suppress measurement of the statement's
total runtime.
Greg Smith
|
|
This simplifies the handling of crashes after fast promotion and various
minor cases that can exist in short timing windows around that case.
Broad fix to bug reported by Michael Paquier on -hackers,
approach prompted by Heikki Linnakangas
|
|
|
|
Michael Paquier
|
|
euc_* and mule_internal test cases were identical to the ones in
src/test/mb. sql_ascii didn't exist elsewhere, but has been broken since
2001, and doesn't seem very interesting anyway. drop.sql hasn't been used
since 2000, when regress.sh was removed.
|
|
Fixes oversight in commit 2ffa740be9d96a3743ecb7e42391c53d0760c65a.
Per report from Josh Kupershmidt.
I think we've broken this case before, so let's add a regression test
this time.
|
|
PathNameOpenFile failed to ensure that the correct value of errno was
returned to its caller after a failure (because it incorrectly supposed
that free() can never change errno). In some cases this would result
in a user-visible failure because an expected ENOENT errno was replaced
with something else. Bogus EINVAL failures have been observed on OS X,
for example.
There were also a couple of places that could mangle an important value
of errno if FDDEBUG was defined. While the usefulness of that debug
support is highly debatable, we might as well make it safe to use,
so add errno save/restore logic to the DO_DB macro.
Per bug #8167 from Nelson Minar, diagnosed by RhodiumToad.
Back-patch to all supported branches.
|
|
If we're going to quote a well-known pangram, we should quote it
accurately. Per gripe from Thom Brown.
|
|
The behavior is that the required sequence is created locally, which is
appropriate because the default expression will be evaluated locally.
Per gripe from Brad Nicholson that this case was refused with a confusing
error message. We could have improved the error message but it seems
better to just allow the case.
Also, remove ALTER TABLE's arbitrary prohibition against being applied to
foreign tables, which was pretty inconsistent considering we allow it for
views, sequences, and other relation types that aren't even called tables.
This is needed to avoid breaking pg_dump, which sometimes emits column
defaults using separate ALTER TABLE commands. (I think this can happen
even when the default is not associated with a sequence, so that was a
pre-existing bug once we allowed column defaults for foreign tables.)
|
|
If OID wraparound should occur while in standalone mode (unlikely but
possible), we want to advance the counter to FirstNormalObjectId not
FirstBootstrapObjectId. Otherwise, user objects might be created with OIDs
in the system-reserved range. That isn't immediately harmful but it poses
a risk of conflicts during future pg_upgrade operations.
Noted by Andres Freund. Back-patch to all supported branches, since all of
them are supported sources for pg_upgrade operations.
|
|
In a construct like "select plain_function(set_returning_function(...))",
the plain function is applied to each output row of the SRF successively.
If some of the SRF outputs are NULL, and the plain function is strict,
you'd expect to get NULL results for such rows ... but what actually
happened was that such rows were omitted entirely from the result set.
This was due to confusion of this case with what should happen for nested
set-returning functions; a strict SRF is indeed supposed to yield an empty
set for null input. Per bug #8150 from Erwin Brandstetter.
Although this has been broken forever, we're not back-patching because
of the possibility that some apps out there expect the incorrect behavior.
This change should be listed as a possible incompatibility in the 9.3
release notes.
|
|
The existing code in NUM_numpart_from_char has hard-wired logic to treat
'.' as decimal point, even when we're using a locale-aware format string
and the locale says that '.' is the thousands separator. This results in
clearly wrong answers in FM mode (where we must be able to identify the
decimal point location), as per bug report from Patryk Kordylewski.
Since the initialization code in NUM_prepare_locale already sets up
Np->decimal as either the locale decimal-point string or "." depending
on which decimal-point format code was used, there's really no need to
have any extra logic at all in NUM_numpart_from_char: we only need to
test for a match to Np->decimal.
(Note: AFAICS there's nothing in here that explicitly checks for thousands
separators --- rather, any unmatched character is silently skipped over.
That's pretty bogus IMO but it's not the issue being complained of.)
This is a longstanding bug, but it's possible that some existing apps
are depending on '.' being recognized as decimal point even when using
a D format code. Hence, no back-patch. We should probably list this
as a potential incompatibility in the 9.3 release notes.
|
|
This case doesn't normally happen, because the planner usually clamps
all row estimates to at least one row; but I found that it can arise
when dealing with relations excluded by constraints. Without a defense,
estimate_num_groups() can return zero, which leads to divisions by zero
inside the planner as well as assertion failures in the executor.
An alternative fix would be to change set_dummy_rel_pathlist() to make
the size estimate for a dummy relation 1 row instead of 0, but that seemed
pretty ugly; and probably someday we'll want to drop the convention that
the minimum rowcount estimate is 1 row.
Back-patch to 8.4, as the problem can be demonstrated that far back.
|
|
Commit d22a09dc70f9830fa78c1cd1a3a453e4e473d354 introduced official support
for GiST consistentFns that want to cache data using the FmgrInfo fn_extra
pointer: the idea was to preserve the cached values across gistrescan(),
whereas formerly they'd been leaked. However, there was an oversight in
that, namely that multiple scan keys might reference the same column's
consistentFn; the code would result in propagating the same cache value
into multiple scan keys, resulting in crashes or wrong answers. Use a
separate array instead to ensure that each scan key keeps its own state.
Per bug #8143 from Joel Roller. Back-patch to 9.2 where the bug was
introduced.
|
|
It is not used anymore.
|
|
Missed in commit 62e666400dddf605b9b6d9a7ac2918711b5c5629.
|
|
This reverts the code changes in 50c137487c96e629e0e5372bb3d1b5f1a2f71a88,
which turned out to induce crashes and not completely fix the problem
anyway. That commit only considered single subqueries that were excluded
by constraint-exclusion logic, but actually the problem also exists for
subqueries that are appendrel members (ie part of a UNION ALL list). In
such cases we can't add a dummy subpath to the appendrel's AppendPath list
without defeating the logic that recognizes when an appendrel is completely
excluded. Instead, fix the problem by having setrefs.c scan the rangetable
an extra time looking for subqueries that didn't get into the plan tree.
(This approach depends on the 9.2 change that made set_subquery_pathlist
generate dummy paths for excluded single subqueries, so that the exclusion
behavior is the same for single subqueries and appendrel members.)
Note: it turns out that the appendrel form of the missed-permissions-checks
bug exists as far back as 8.4. However, since the practical effect of that
bug seems pretty minimal, consensus is to not attempt to fix it in the back
branches, at least not yet. Possibly we could back-port this patch once
it's gotten a reasonable amount of testing in HEAD. For the moment I'm
just going to revert the previous patch in 9.2.
|
|
If a standby server has a cascading standby server connected to it, it's
possible that WAL has already been sent up to the next WAL page boundary,
splitting a WAL record in the middle, when the first standby server is
promoted. Don't throw an assertion failure or error in walsender if that
happens.
Also, fix a variant of the same bug in pg_receivexlog: if it had already
received WAL on previous timeline up to a segment boundary, when the
upstream standby server is promoted so that the timeline switch record falls
on the previous segment, pg_receivexlog would miss the segment containing
the timeline switch. To fix that, have walsender send the position of the
timeline switch at end-of-streaming, in addition to the next timeline's ID.
It was previously assumed that the switch happened exactly where the
streaming stopped.
Note: this is an incompatible change in the streaming protocol. You might
get an error if you try to stream over timeline switches, if the client is
running 9.3beta1 and the server is more recent. It should be fine after a
reconnect, however.
Reported by Fujii Masao.
|
|
What we have implemented is a radix tree (or a radix trie or a patricia
trie), but the docs and code comments incorrectly called it a "suffix tree".
Alexander Korotkov
|
