| Age | Commit message (Collapse) | Author |
|---|
|
Large objects are unsupported on Postgres-XL, so the failures are
expected.
|
|
The plan changes come from the upstream, and XL only adds Remote Fast
Query Execution at the top.
|
|
The old-style C functions do not exist anymore, so remove the block of
code testing them (and failing). This test was removed from upstream,
and was only kept by mistake during the merge.
|
|
The failures were fairly trivial in nature:
* extra output for test block removed in PostgreSQL 10
* non-deterministic ordering of results
* functions referencing temporary tables, which does not work on XL
* triggers not supported on XL
Resolving the first two issue is simple - remove the extra blocks and
add ORDER BY to stabilize the ordering.
To fix the temp tables vs. functions issue I've simply made all the
tables non-temporary. The triggers were used merely to generate some
notices, so removing those from the expected output was enough.
|
|
The output routine for anyarray data type (anyarray_out) is modified to
include name of the data type in the text representation. Which is also
used when an error message includes the value, for example duplicate
key errors.
|
|
|
|
This change didn't adjust the publicly visible taptest function, causing
buildfarm failures on bowerbird.
Backpatch to 9.4 like previous change.
|
|
Postgres-XL does not support SAVEPOINT (or subtransactions in general).
Some regression tests use this to test cases that are expected to fail,
and we want to keep testing that. So instead of removing the tests,
split them into independent transactions (and tweak the expected output
accordingly).
Note: The tests are still failing, though. Apparently XL does not undo
the effect of ALTER TYPE ... ADD VALUE on rollback.
|
|
Mostly just renames of the objects, received from upstream.
|
|
Given this code's general eagerness to use subexpressions' output variables
as temporary workspace, it's not exactly clear that it is safe for
FieldStore to tell a newval subexpression that it can write into the same
variable that is being supplied as a potential input. Document the chain
of assumptions needed for that to be safe.
|
|
I got confused about why this function doesn't need to recursively
search the expression tree for a CaseTestExpr node. After figuring
that out, add a comment to save the next person some time.
|
|
The plans are fairly close to those generated on Postgres-XL 9.5, with
some minor differences due to upstream optimizer changes (e.g. missing
Subquery Scan when we can do Index Scan instead).
The main change is that when a Merge Join is identified as unique, we
may replace
-> Materialize
-> Sort
Sort Key: ec1.f1 USING <
-> Remote Subquery Scan on all (datanode1)
-> Index Scan using ec1_pkey on ec1
Index Cond: (ff = '42'::bigint)
with
-> Remote Subquery Scan on all (datanode_1)
-> Sort
Sort Key: ec1.f1 USING <
-> Index Scan using ec1_pkey on ec1
Index Cond: (ff = '42'::bigint)
as there will be no rescans on the inner relation (so we do not need
the additional Materialize step).
|
|
Accept simple plan changes in tsearch regression test
The accepted plan changes are fairly simple, switching plain aggregation
to a distributed one.
|
|
Grouping sets are stricter about distribution of input data, as all the
execution happens on the coordinator - there is no support for partial
grouping sets yet, so we can either push all the grouping set work to
the remote node (if all the sets include the distribution key), or make
sure that there is a Remote Subquery on the input path.
This is what Postgres-XL 9.6 was doing, but it got lost during merge
with PostgreSQL 10 which significantly reworked this part of the code.
Two queries still produce incorrect result, but those are not actually
using the grouping sets paths because
GROUP BY GROUPING SETS (a, b)
gets transformed into simple
GROUP BY a, b
and ends up using parallel aggregation. The bug seems to be that the
sort orders mismatch for some reason - the remote part produces data
sorted by "a" but the "Finalize GroupAggregate" expects input sorted
by "a, b" leading to duplicate groups in the result.
|
|
The upstream privileges regression test added multiple checks of explain
plans, so the plans needed to be adjusted for Postgres-XL (by adding the
Remote Subquery nodes to appropriate places).
There are two plans that however mismatch the upstream version, using
a different join algorithm (Nested Loop vs. Hash Join). Turns out this
happens due to Postgres-XL not collecting stats for expression indexes,
and the two queries rely on that feature. Without the statistics the
estimates change dramatically, triggering a plan change.
We need to extend analyze_rel_coordinator() to collect stats not only
for the table, but for all indexes too. But that's really a matter for
a separate commit.
|
|
Trivial result ordering stabilization by adding ORDER BY clause, and
accepting explain output with additional Remote Subquery node (on top
of the upstream plan).
|
|
Stabilized by adding ORDER BY clause to two queries. Generate explain
plans both for the original and modified queries.
|
|
The expected output was missing block testing ON TRUNCATE triggers. Add
it and tweak to reflect Postgres-XL limitations (no triggers or RESTART
IDENTITY).
There seems to be an issue in handling sequences with START WITH clause,
where we don't quite respet that and start with a higher value. And we
also don't increment by 1 for some reason.
|
|
The original wording was impossible to translate correctly.
Discussion: https://postgr.es/m/20170523002827.lzc2jkzh2gubclqb@alvherre.pgsql
|
|
Ordering of some results in xc_FQS tests became unstable, so stabilize
it by adding ORDER BY clauses. Instead of just changing the explain
plans in the same way, generate plans both for the original query
(without the ORDER BY clause) and the new one.
|
|
The join is detected to be unique, which is indicated by 'Inner Unique'
key in the explain plan.
|
|
Upstream commit 5c80642aa8de8393b08cd3cbf612b325cedd98dc removed support
for hashing int2vector data type, as it was dead code (upstream). That
means we can no longer distribute table by hash on this datatype.
We could reintroduce the hash function in Postgres-XL, but int2vector
seems rarely used as distribution key, so let's just fix the tests. If
needed, we can add the functions in the future.
|
|
Add missing infrastructure for this node type, notably in ruleutils.c where
its lack could demonstrably cause EXPLAIN to fail. Add outfuncs/readfuncs
support. (outfuncs support is useful today for debugging purposes. The
readfuncs support may never be needed, since at present it would only
matter for parallel query and NextValueExpr should never appear in a
parallelizable query; but it seems like a bad idea to have a primnode type
that isn't fully supported here.) Teach planner infrastructure that
NextValueExpr is a volatile, parallel-unsafe, non-leaky expression node
with cost cpu_operator_cost. Given its limited scope of usage, there
*might* be no live bug today from the lack of that knowledge, but it's
certainly going to bite us on the rear someday. Teach pg_stat_statements
about the new node type, too.
While at it, also teach cost_qual_eval() that MinMaxExpr, SQLValueFunction,
XmlExpr, and CoerceToDomain should be charged as cpu_operator_cost.
Failing to do this for SQLValueFunction was an oversight in my commit
0bb51aa96. The others are longer-standing oversights, but no time like the
present to fix them. (In principle, CoerceToDomain could have cost much
higher than this, but it doesn't presently seem worth trying to examine the
domain's constraints here.)
Modify execExprInterp.c to execute NextValueExpr as an out-of-line
function; it seems quite unlikely to me that it's worth insisting that
it be inlined in all expression eval methods. Besides, providing the
out-of-line function doesn't stop anyone from inlining if they want to.
Adjust some places where NextValueExpr support had been inserted with the
aid of a dartboard rather than keeping it in the same order as elsewhere.
Discussion: https://postgr.es/m/23862.1499981661@sss.pgh.pa.us
|
|
In the frontend Makefiles that pull in libpgfeutils, we'd generally
done it like this:
LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
That method is badly broken, as seen in bug #14742 from Chris Ruprecht.
The -L flag for src/fe_utils ends up being placed after whatever random
-L flags are in LDFLAGS already. That puts us at risk of pulling in
libpgfeutils.a from some previous installation rather than the freshly
built one in src/fe_utils. Also, the lack of an "override" is hazardous
if someone tries to specify some LDFLAGS on the make command line.
The correct way to do it is like this:
override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
so that libpgfeutils, along with libpq, libpgport, and libpgcommon, are
guaranteed to be pulled in from the build tree and not from any referenced
system directory, because their -L flags will appear first.
In some places we'd been even lazier and done it like this:
LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq
which is subtly wrong in an additional way: on platforms where we can't
restrict the symbols exported by libpq.so, it allows libpgfeutils to
latch onto libpgport and libpgcommon symbols from libpq.so, rather than
directly from those static libraries as intended. This carries hazards
like those explained in the comments for the libpq_pgport macro.
In addition to fixing the broken libpgfeutils usages, I tried to
standardize on using $(libpq_pgport) like so:
override LDFLAGS := $(libpq_pgport) $(LDFLAGS)
even where libpgfeutils is not in the picture. This makes no difference
right now but will hopefully discourage future mistakes of the same ilk.
And it's more like the way we handle CPPFLAGS in libpq-using Makefiles.
In passing, just for consistency, make pgbench include PTHREAD_LIBS the
same way everyplace else does, ie just after LIBS rather than in some
random place in the command line. This might have practical effect if
there are -L switches in that macro on some platform.
It looks to me like the MSVC build scripts are not affected by this
error, but someone more familiar with them than I might want to double
check.
Back-patch to 9.6 where libpgfeutils was introduced. In 9.6, the hazard
this error creates is that a reinstallation might link to the prior
installation's copy of libpgfeutils.a and thereby fail to absorb a
minor-version bug fix.
Discussion: https://postgr.es/m/20170714125106.9231.13772@wrigleys.postgresql.org
|
|
When writing a backup to stdout with pg_basebackup on Windows, put stdout
to binary mode. Any CR bytes in the output will otherwise be output
incorrectly as CR+LF.
In the passing, standardize on using "_setmode" instead of "setmode", for
the sake of consistency. They both do the same thing, but according to
MSDN documentation, setmode is deprecated.
Fixes bug #14634, reported by Henry Boehlert. Patch by Haribabu Kommi.
Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/20170428082818.24366.13134@wrigleys.postgresql.org
|
|
The grammar will only accept something syntactically similar to a function
call in a function-in-FROM expression. However, there are various ways
to input something that ruleutils.c won't deparse that way, potentially
leading to a view or rule that fails dump/reload. Fix by inserting a
dummy CAST around anything that isn't going to deparse as a function
(which is one of the ways to get something like that in there in the
first place).
In HEAD, also make use of the infrastructure added by this to avoid
emitting unnecessary parentheses in CREATE INDEX deparsing. I did
not change that in back branches, thinking that people might find it
to be unexpected/unnecessary behavioral change.
In HEAD, also fix incorrect logic for when to add extra parens to
partition key expressions. Somebody apparently thought they could
get away with simpler logic than pg_get_indexdef_worker has, but
they were wrong --- a counterexample is PARTITION BY LIST ((a[1])).
Ignoring the prettyprint flag for partition expressions isn't exactly
a nice solution anyway.
This has been broken all along, so back-patch to all supported branches.
Discussion: https://postgr.es/m/10477.1499970459@sss.pgh.pa.us
|
|
The new functions return a list of files in the corresponding directory,
not the name of the directory itself.
Pointed out by Gianni Ciolli.
|
|
ANALYZE throws a WARNING that extended statistics was not built due to
statistics target being 0 for some of the columns, but that gets thrown
on the datanode and we never forward it to the coordinator.
So explicitly check contents of pg_statistic_ext if the statistic was
built or not.
|
|
The format used by psql \d and \d+ changed a bit, splitting the single
Modifiers column into Collation, Nullable, Default. Additional commands
changed too, for example \dew+ now uses "options" instead of "Options"
and so on.
This commit accepts all such output changes across all regression tests.
|
|
The regression test was failing because many queries started producing
results with unstable ordering, so fix that by adding ORDER BY clause
to many of them.
This of course affects the plans that were part of the test too, so
instead of running query with ORDER BY clause and then checking plan
for a query without it, check plans for both query versions. This makes
the test somewhat bigger, but it seems to be worth it and the impact on
test duration is negligible.
Multiple query plans changed in a non-trivial ways, too. This commit
accepts changes that are clearly inherited from the upstream, which was
verified by running the query on PostgreSQL 10 and accepting simple
changes (essentially adding "Remote Subquery" to reasonable places in
the plan or "Remote Fast Query Execution" at the top).
More complicated plan changes (e.g. switching from Group Aggregate to
Hash Aggregate or back, join algorithms etc.) are left unaccepted for
additional analysis.
The SQL script also generates multiple query plans that are not included
in the expected output. This is intentional, as the generated plans are
incorrect and produce incorrect ordering of results. The bug is that
queries like
SELECT sum(a) FROM t GROUP BY 1
end up producing results sorted by 'a' and not 'sum(a)'.
|
|
The rowsecurity test suite hits unsupported features on two places:
* WHERE CURRENT OF clause for cursors
* SAVEPOINTS (subtransactions)
which results in 'current transaction is aborted' errors in the rest of
the transaction. Those errors were added to the expected output file,
making the regression test succeed, but it may easily mask issues in the
aborted part of the transaction.
We need to rework the rest so that it skips the unsupported features,
and still exercises the test on the remaining parts.
|
|
This is a mix of simple fixes to stabilize the rowsecurity test.
1) Adding ORDER BY to multiple queries, to stabilize the output order.
2) Update expected query output by copying it from upstream (PostgreSQ).
Apparently some of this got broken during merge conflict resolution.
3) Accept simple plan changes that add Remote Subquery either at the top
of the plan, or in InitPlan / SubPlan or CTE.
4) Accept plan changes inherited from upstream, particularly removal of
the "Subquery Scan" nodes from the plan.
5) Add expected output for "viewpoint from regress_rls_dave" section,
missing in the expected file for some reason.
|
|
All accepted plan changes are simply adding Remote Subquery at the top
of a plan merged from upstream, in a fairly obviously correct way.
There are two additional fixes, either adding a missing block of
expected output (copied from upstream), or removing an extra output.
|
|
When running ANALYZE on a coordinator, we simply fetch the statistics
built on datanodes, and keep stats from a random datanode (assuming all
datanodes are similar in terms of data volume and data distribution).
This was only done for regular per-attribute stats, though, not for the
extended statistics added in PostgreSQL 10, causing various failures in
stats_ext tests due to missing statistics. This commit fixes this gap
by using the same approach as for simple statistics - we collect stats
from datanodes and keep the first result we receive for each statistic.
While working on this I realized this approach has some inherent issues,
particularly on columns that are distribution keys. As we keep stats
from a random node, we completely ignore MCV and histograms from the
remaining nodes. That may cause planning issues, but addressing it is
out of scope for this commit.
|
|
The race condition goes like this:
1. GetNewTransactionId advances nextXid e.g. from 100 to 101
2. GetOldestActiveTransactionId reads the new nextXid, 101
3. GetOldestActiveTransactionId loops through the proc array. There are no
active XIDs there, so it returns 101 as the oldest active XID.
4. GetNewTransactionid stores XID 100 to MyPgXact->xid
So, GetOldestActiveTransactionId returned XID 101, even though 100 only
just started and is surely still running.
This would be hard to hit in practice, and even harder to spot any ill
effect if it happens. GetOldestActiveTransactionId is only used when
creating a checkpoint in a master server, and the race condition can only
happen on an online checkpoint, as there are no backends running during a
shutdown checkpoint. The oldestActiveXid value of an online checkpoint is
only used when starting up a hot standby server, to determine the starting
point where pg_subtrans is initialized from. For the race condition to
happen, there must be no other XIDs in the proc array that would hold back
the oldest-active XID value, which means that the missed XID must be a top
transaction's XID. However, pg_subtrans is not used for top XIDs, so I
believe an off-by-one error is in fact inconsequential. Nevertheless, let's
fix it, as it's clearly wrong and the fix is simple.
This has been wrong ever since hot standby was introduced, so backport to
all supported versions.
Discussion: https://www.postgresql.org/message-id/e7258662-82b6-7a45-56d4-99b337a32bf7@iki.fi
|
|
This merge includes all commits upto bc2d716ad09fceeb391c755f78c256ddac9d3b9f
of PG 10.
|
|
For partitioned tables or in general inherited tables, we now enforce that the
child table always inherit the distribution strategy of the parent. This not
only makes it far easier to handle various cases correctly, but would also
allow us to optimise distributed queries on partitioned tables much easily.
Tank.zhang <6220104@qq.com> originally reported a problem with partitioned
tables and incorrect query execution. Upon investigations, we decided to make
these restrictions to simplify things.
|
|
Further investigation shows that ruleutils isn't quite up to speed either
for cases where we have a domain-over-array: it needs to be prepared to
look past a CoerceToDomain at the top level of field and element
assignments, else it decompiles them incorrectly. Potentially this would
result in failure to dump/reload a rule, if it looked like the one in the
new test case. (I also added a test for EXPLAIN; that output isn't broken,
but clearly we need more test coverage here.)
Like commit b1cb32fb6, this bug is reachable in cases we already support,
so back-patch all the way.
|
|
compute_tsvector_stats() detoasted and kept in memory every tsvector value
in the sample, but that can be a lot of memory. The original bug report
described a case using over 10 gigabytes, with statistics target of 10000
(the maximum).
To fix, allocate a separate copy of just the lexemes that we keep around,
and free the detoasted tsvector values as we go. This adds some palloc/pfree
overhead, when you have a lot of distinct lexemes in the sample, but it's
better than running out of memory.
Fixes bug #14654 reported by James C. Reviewed by Tom Lane. Backport to
all supported versions.
Discussion: https://www.postgresql.org/message-id/20170514200602.1451.46797@wrigleys.postgresql.org
|
|
Otherwise, the script output has a lot of pointless warnings.
This was forgotten in 9def031bd2821f35b5f506260d922482648a8bb0
|
|
If the number of tuples in the heap exceeds approximately INT_MAX/2,
this loop's calculation "2*i+1" could overflow, resulting in a crash.
Fix it by using unsigned int rather than int for the relevant local
variables; that shouldn't cost anything extra on any popular hardware.
Per bug #14722 from Sergey Koposov.
Original patch by Sergey Koposov, modified by me per a suggestion
from Heikki Linnakangas to use unsigned int not int64.
Back-patch to 9.4, where tuplesort.c grew the ability to sort as many
as INT_MAX tuples in-memory (commit 263865a48).
Discussion: https://postgr.es/m/20170629161637.1478.93109@wrigleys.postgresql.org
|
|
This addresses simple failures in stats_ext regression tests, namely:
1) Failure to drop column that happens to be a distribution key of the
table (picked automatically by CREATE TABLE). Resolved by explicitly
distributing the table by another columns.
2) Commenting out DDL on FDWs, which are unsupported in Postgres-XL, and
we already have this covered by plenty of other tests.
3) Commenting out a DO block, checking that we can't create statistics
on a TOAST table. The error message will vary depending on what OID
is assigned to the TOAST table. There's no nice way to catch that
error in Postgres-XL, due to missing support for subtransactions.
4) Accept trivial plan changes, that simply add Remote Subquery into the
plan, or ship the whole query to the nodes (Fast Query Execution).
There additional plan changes that change the plans in other ways,
but those need more investigation and are not accepted by this commit.
|
|
Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/20170711.163441.241981736.horiguchi.kyotaro@lab.ntt.co.jp
|
|
Commit 14e8803f1 removed the locking in SyncRepWaitForLSN, but that
introduced a race condition, where SyncRepWaitForLSN might see
syncRepState already set to SYNC_REP_WAIT_COMPLETE, but the process was
not yet removed from the queue. That tripped the assertion, that the
process should no longer be in the uqeue. Reorder the operations in
SyncRepWakeQueue to remove the process from the queue first, and update
syncRepState only after that, and add a memory barrier in between to make
sure the operations are made visible to other processes in that order.
Fixes bug #14721 reported by Const Zhang. Analysis and fix by Thomas Munro.
Backpatch down to 9.5, where the locking was removed.
Discussion: https://www.postgresql.org/message-id/20170629023623.1480.26508%40wrigleys.postgresql.org
|
|
Mostly in the new subscription-related commands. Backport the few that
were also present in older versions.
Thomas Munro
Discussion: https://www.postgresql.org/message-id/CAEepm=3CyW1QmXcXJXmqiJXtXzFDc8SvSfnxkEGD3Bkv2SrkeQ@mail.gmail.com
|
|
We allow INSERT and UPDATE commands to assign to the same column more than
once, as long as the assignments are to subfields or elements rather than
the whole column. However, this failed when the target column was a domain
over array rather than plain array. Fix by teaching process_matched_tle()
to look through CoerceToDomain nodes, and add relevant test cases.
Also add a group of test cases exercising domains over array of composite.
It's doubtless accidental that CREATE DOMAIN allows this case while not
allowing straight domain over composite; but it does, so we'd better make
sure we don't break it. (I could not find any documentation mentioning
either side of that, so no doc changes.)
It's been like this for a long time, so back-patch to all supported
branches.
Discussion: https://postgr.es/m/4206.1499798337@sss.pgh.pa.us
|
|
To optimise and simplify XL's distributed query planning, we enforce that all
partitions of a partitioned table use the same distribution strategy. We also
put further restrictions that all columns in the partitions and the partitioned
table has matching positions. This can cause some problems when tables have
dropped columns etc, but we think it's far better to optimise XL's plans than
supporting all corner cases. We can look at removing some of these
restrictions later once the more usual queries run faster.
These restrictions allow us to unconditionally push down Append and MergeAppend
nodes to datanodes when these nodes are processing partitioned tables.
Some regression tests currently fail because of these added restrictions. We
would look at them in due course of time.
|
|
|
|
Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: c5a8de3653bb1af6b0eb41cc6bf090c5522df52b
|
|
We've heard occasional reports of backend launch failing because
pgwin32_ReserveSharedMemoryRegion() fails, indicating that something
has already used that address space in the child process. It's not
very clear what, given that we disable ASLR in Windows builds, but
suspicion falls on antivirus products. It'd be better if we didn't
have to disable ASLR, anyway. So let's try to ameliorate the problem
by retrying the process launch after such a failure, up to 100 times.
Patch by me, based on previous work by Amit Kapila and others.
This is a longstanding issue, so back-patch to all supported branches.
Discussion: https://postgr.es/m/CAA4eK1+R6hSx6t_yvwtx+NRzneVp+MRqXAdGJZChcau8Uij-8g@mail.gmail.com
|
