postgres-xl.git
6 years agoFix incorrect comparison in pgxcnode_gethash master
Tomas Vondra [Fri, 12 Oct 2018 12:32:54 +0000 (14:32 +0200)]
Fix incorrect comparison in pgxcnode_gethash

The check is supposed to ensure NULL/empty nodename gets hashed to 0,
but (nodename == '\0') is comparing the pointer itself, not the first
character. So dereference that correctly.

6 years agoUse sufficiently large buffer in SharedQueueWrite
Tomas Vondra [Fri, 12 Oct 2018 12:23:29 +0000 (14:23 +0200)]
Use sufficiently large buffer in SharedQueueWrite

The sq_key alone may be up to 64 bytes, so we need more than that.
We could use dynamic memory instead, but 128 bytes should be enough
both for the sq_key and the other pieces.

6 years agoUse dynamic buffer to parse NODE_LIST_RESULT in GTM
Tomas Vondra [Thu, 11 Oct 2018 14:51:09 +0000 (16:51 +0200)]
Use dynamic buffer to parse NODE_LIST_RESULT in GTM

When processing NODE_LIST_RESULT messages, gtmpqParseSuccess() used
a static buffer, defined as "char buf[8092]".  This is an issue, as
the message has variable length, and may get long enough to exceed
any hard-coded limit.  While that's not very common (it requires
long paths, node names and/or many GTM sessions on the node), it
may happen, in which case the memcpy() causes a buffer overflow and
corrupts the stack.

Fixing this is simple - allocate the buffer using malloc() intead,
requesting exactly the right amount of memory.  This however hits
a latent pre-existing issue in the code, because the code was doing
memcpy(&buf,...) instead of memcpy(buf,...).  With static buffers
this was harmless, because (buf == &buf), so the code was working
as intended (except when there were more than 8092 bytes).  With
dynamic memory this is no longer true, becase (buf != &buf), and
the stack corruption was much easier to trigger (just 8 bytes).

Per report and debug info by Hengbing. Patch by Pavan and me.

6 years agoUse correct path for tablspaces while creating a basebackup
Pavan Deolasee [Fri, 3 Aug 2018 08:39:12 +0000 (14:09 +0530)]
Use correct path for tablspaces while creating a basebackup

In XL, we embed the nodename in the tablespace subdir name to ensure that
non-conflicting paths are created when multiple coordinators/datanodes are
running on the same server. The code to handle tablespace mapping in basebackup
was missing this support.

Per report and patch by Wanglin.

6 years agoFix compilation error introduced in 0e3770c14c3fb1858192feb7240343cb35ba013c
Pavan Deolasee [Fri, 3 Aug 2018 04:18:18 +0000 (09:48 +0530)]
Fix compilation error introduced in 0e3770c14c3fb1858192feb7240343cb35ba013c

Per report and patch by Wanglin

6 years agoEnsure that bad protocol ERROR message is sent to the frontend
Pavan Deolasee [Tue, 31 Jul 2018 06:00:01 +0000 (11:30 +0530)]
Ensure that bad protocol ERROR message is sent to the frontend

In case of receiving bad protocol messages received by the GTM proxy, let
the client know about the error messages.

6 years agoCorrect select the GTM proxy for a new node being added
Pavan Deolasee [Tue, 31 Jul 2018 05:58:21 +0000 (11:28 +0530)]
Correct select the GTM proxy for a new node being added

This fixes an oversight in array index lookup. We should have been using
0-based indexes but were instead using 1-based index.

6 years agoEnsure partition child tables inherit distribution properties correctly
Pavan Deolasee [Mon, 30 Jul 2018 08:47:20 +0000 (14:17 +0530)]
Ensure partition child tables inherit distribution properties correctly

While in restore mode, that we use to load schema when a new node is added to
the cluster, the partition child tables should correctly inherit the
distribution properties from the parent table. This support was lacking, thus
leading to incorrect handling of such tables.

Per report by Virendra Kumar.

6 years agoDo not dump TO NODE clause for partition or child table
Pavan Deolasee [Fri, 27 Jul 2018 12:19:38 +0000 (17:49 +0530)]
Do not dump TO NODE clause for partition or child table

We missed this in the commit c168cc8d58c6e0d9710ef0aba1b846b7174e0a79. So deal
with it now.

6 years agoEnsure qualified name for dumping sequence value
Pavan Deolasee [Fri, 27 Jul 2018 07:51:51 +0000 (13:21 +0530)]
Ensure qualified name for dumping sequence value

Without that the sequence won't be found correctly.

6 years agoDo not dump DISTRIBUTED BY for partition and inherited table
Pavan Deolasee [Fri, 27 Jul 2018 06:59:13 +0000 (12:29 +0530)]
Do not dump DISTRIBUTED BY for partition and inherited table

Child tables inherit the distribition property from the parent table. Even
more, XL doesn't support a syntax of the form PARTITION OF .. DISTRIBUTED BY
and doesn't allow child tables to have a distribution property different than
the parent. So attaching this clause to the partition table does not make any
sense.

Per report from Virendra Kumar.

6 years agoTeach pgxc_exec_sizefunc() to use pg_my_temp_schema() to get temp schema
Pavan Deolasee [Thu, 19 Jul 2018 09:31:07 +0000 (15:01 +0530)]
Teach pgxc_exec_sizefunc() to use pg_my_temp_schema() to get temp schema

Similar to what we did in e688c0c23c962d425b82fdfad014bace4207af1d, we must not
rely on the temporary namespace on the coordinator since it may change on the
remote nodes. Instead we use the pg_my_temp_schema() function to find the
currently active temporary schema on the remote node.

6 years agoTeach pgxc_ctl to use the new --waldir option of pg_basebackup
Pavan Deolasee [Tue, 17 Jul 2018 04:56:50 +0000 (10:26 +0530)]
Teach pgxc_ctl to use the new --waldir option of pg_basebackup

PG 10 replaced --xlogdir with --waldir, but we forgot to update pgxc_ctl to use
the new syntax. This patch fixes that oversight.

Per report and analysis by Virendra Kumar and patch by Mark Wong.

6 years agoFix handling of REFRESH MATERIALIZED VIEW CONCURRENTLY
Pavan Deolasee [Tue, 17 Jul 2018 04:47:40 +0000 (10:17 +0530)]
Fix handling of REFRESH MATERIALIZED VIEW CONCURRENTLY

We create a coordinator-only LOCAL temporary table for REFRESH MATERIALIZED
VIEW CONCURRENTLY. Since this table does not exist on the remote nodes, we must
not use explicit "ANALYZE <temptable>". Instead, just analyze it locally like
we were doing at other places.

Restore the matview test case to use REFRESH MATERIALIZED VIEW CONCURRENTLY now
that the underlying bug is fixed.

6 years agoImprove locking semantics in GTM and GTM Proxy
Pavan Deolasee [Tue, 10 Jul 2018 16:12:16 +0000 (21:42 +0530)]
Improve locking semantics in GTM and GTM Proxy

While GTM allows long jump in case of errors, we were not careful to release
locks currently held by the executing thread. That could lead to threads
leaving a critical section still holding a lock and thus causing deadlocks.

We now properly track currently held locks in the thread-specific information
and release those locks in case of an error. Same is done for mutex locks as
well, though there is only one that gets used.

This change required using a malloc-ed memory for thread-specific info. While
due care has been taken to free the structure, we should keep an eye on it for
any possible memory leaks.

In passing also improve handling of bad-protocol startup messages which may
have caused deadlock and resource starvation.

6 years agoFix a compiler warning introduced in the previous commit
Pavan Deolasee [Tue, 10 Jul 2018 12:23:18 +0000 (17:53 +0530)]
Fix a compiler warning introduced in the previous commit

6 years agoEnsure that typename is schema qualified while sending row description
Pavan Deolasee [Tue, 10 Jul 2018 09:10:56 +0000 (14:40 +0530)]
Ensure that typename is schema qualified while sending row description

A row description messages contains the type information for the attributes in
the column. But if the type does not exist in the search_path then the
coordinator fails to parse the typename back to the type. So the datanode must
send the schema name along with the type name.

Per report and test case by Hengbing Wang @ Microfun.

Added a new test file and a few test cases to cover this area.

6 years agoEnsure pooler process follows consistent model for SIGQUIT handling
Pavan Deolasee [Mon, 18 Jun 2018 09:16:08 +0000 (14:46 +0530)]
Ensure pooler process follows consistent model for SIGQUIT handling

We'd occassionally seen that the pooler process fails to respond to SIGQUIT and
gets stuck in a non recoverable state. Code inspection reveals that we're not
following the model followed by rest of the background worker processes in
handling SIGQUIT. So get that fixed, with the hope that this will fix the
problem case.

6 years agoProperly quote typename before calling parseTypeString
Pavan Deolasee [Mon, 18 Jun 2018 09:14:08 +0000 (14:44 +0530)]
Properly quote typename before calling parseTypeString

Without this, parseTypeString() might throw an error or resolve to a wrong type
in case the type name requires quoting.

Per report by Hengbing Wang

6 years agoRemove some accidentally added elog(LOG) messages
Pavan Deolasee [Mon, 21 May 2018 06:41:40 +0000 (12:11 +0530)]
Remove some accidentally added elog(LOG) messages

6 years agoFix broken implementation of recovery to barrier.
Pavan Deolasee [Fri, 18 May 2018 09:30:36 +0000 (15:00 +0530)]
Fix broken implementation of recovery to barrier.

Per report from Hengbing, the current implementation of PITR recovery to a
BARRIER failed to correctly stop at the given recovery_target_barrier. It seems
there are two bugs here. 1) we failed to write the XLOG record correctly and 2)
we also failed to mark the end-of-recovery upon seeing the XLOG record during
the recovery.

Fix both these problems and also fix pg_xlogdump in passing to ensure we can
dump the BARRIER XLOG records correctly.

6 years agoFix a long standing bug in vacuum/analyze of temp tables
Pavan Deolasee [Fri, 18 May 2018 06:16:17 +0000 (11:46 +0530)]
Fix a long standing bug in vacuum/analyze of temp tables

The system may and very likely choose different namespace for temporary tables
on different nodes. So it was erroneous to explicitly add the coordinator side
nampspace to the queries constructed for fetching stats from the remote nodes.
A regression test was non-deterministically failing for this reason for long,
but only now we could fully understand the problem and fix it. We now use
pg_my_temp_schema() to derive the current temporary schema used by the remote
node instead of hardcoding that in the query using coordinator side
information.

6 years agoAccept regression diffs in join test case
Pavan Deolasee [Tue, 15 May 2018 12:25:35 +0000 (17:55 +0530)]
Accept regression diffs in join test case

The plans now look the same as vanilla PG except for additional Remote Fast
Query Execution nodes

6 years agoAccept regression diffs in plpgsql test case
Pavan Deolasee [Mon, 21 May 2018 06:21:42 +0000 (11:51 +0530)]
Accept regression diffs in plpgsql test case

The new output looks correct and has been fixed because of our work to get
transaction handling correct.

6 years agoAdd expected changes to plpgsql.out missed in 0f65a7193da4b6b0a35b6446b4c904a9f5ac9bf6
Pavan Deolasee [Mon, 21 May 2018 06:19:18 +0000 (11:49 +0530)]
Add expected changes to plpgsql.out missed in 0f65a7193da4b6b0a35b6446b4c904a9f5ac9bf6

6 years agoAccept regression diff.
Pavan Deolasee [Tue, 15 May 2018 10:30:12 +0000 (16:00 +0530)]
Accept regression diff.

We no longer see "DROP INDEX CONCURRENTLY cannot run inside a transaction
block" if the index does not exists and we're running DROP IF EXISTS
command

6 years agoFix post-cherry-pick problems.
Pavan Deolasee [Fri, 18 May 2018 11:40:16 +0000 (17:10 +0530)]
Fix post-cherry-pick problems.

6 years agoTrack clearly whether to run a remote transaction in autocommit or a block
Pavan Deolasee [Mon, 9 Apr 2018 10:42:54 +0000 (16:12 +0530)]
Track clearly whether to run a remote transaction in autocommit or a block

Chi Gao and Hengbing Wang reported certain issues around transaction handling
and demonstrated via xlogdump how certain transactions were getting marked
committed/aborted repeatedly on a datanode. When an already committed
transaction is attempted to be aborted again, it results in a PANIC. Upon
investigation, this uncovered a very serious yet long standing bug in
transaction handling.

If the client is running in autocommit mode, we try to avoid starting a
transaction block on the datanode side if only one datanode is going to be
involved in the transaction. This is an optimisation to speed up short queries
touching only a single node. But when the query rewriter transforms a single
statement into multiple statements, we would still (and incorrectly) run each
statement in an autocommit mode on the datanode. This can cause inconsistencies
when one statement commits but the next statement aborts. And it may also lead
to the PANIC situations if we continue to use the same global transaction
identifier for the statements.

This can also happen when the user invokes a user-defined function. If the
function has multiple statements, each statement will run in an autocommit
mode, if it's FQSed, thus again creating inconsistency if a following statement
in the function fails.

We now have a more elaborate mechanism to tackle autocommit and transaction
block needs. The special casing for force_autocommit is now removed, thus
making it more predictable. We also have specific conditions to check to ensure
that we don't mixup autocommit and transaction block for the same global xid.
Finally, if a query rewriter transforms a single statement into multiple
statements, we run those statements in a transaction block. Together these
changes should help us fix the problems.

7 years agoDo not try to show targetlist of a RemoteSubplan on top of ModifyTable
Pavan Deolasee [Mon, 7 May 2018 04:54:21 +0000 (10:24 +0530)]
Do not try to show targetlist of a RemoteSubplan on top of ModifyTable

We do some special processing for RemoteSubplan with returning lists. But the
EXPLAIN plan mechanism is not adequetly trained to handle that special
crafting. So for now do not try to print the target list in the EXPLAIN output.

7 years agoDo not send the new protocol message to non-XL client.
Pavan Deolasee [Mon, 9 Apr 2018 11:35:13 +0000 (17:05 +0530)]
Do not send the new protocol message to non-XL client.

The new message 'W' to report waited-for XIDs must not be sent to a non-XL
client since it's not capable of handling that and might just cause unpleasant
problems. In fact, we should change 'W' to something else since standard libpq
understands that message and hangs forever expecting more data. With a new
protocol message, it would have failed, thus providing a more user friend
error. But postponing that for now since we should think through implications
of protocol change carefully before doing that.

7 years agoUse local variable to format timestamp in GTM log line prefix
Tomas Vondra [Sun, 12 Nov 2017 11:01:53 +0000 (12:01 +0100)]
Use local variable to format timestamp in GTM log line prefix

When formatting log line prefix in GTM, we can't use global variable,
because multiple threads may scribble over the same value. This is
why the timestamp was missing in some log lines - one thread did the
strftime(), but before it used the value another thread truncated the
string (which is the first step in formatting a log line).

So instead use a local (not shared by threads) variable, and pass it
to setup_formatted_log_time() explicitly.

7 years agoFix bug in release_connection() introduced by d9f45c9018
Tomas Vondra [Tue, 7 Nov 2017 17:34:55 +0000 (18:34 +0100)]
Fix bug in release_connection() introduced by d9f45c9018

d9f45c9018ec3ec1fc11e4be2be7f9728a1799b1 attempted to refactor
release_connection() to make it more readable, but unfortunately
inverted the force_destroy check, causing regression failures.

In hindsight, the refactoring was rather arbitrary and not really
helping with the readability, so just revert to the original code
(but keep the comments, explaining what's happening).

7 years agoMove several functions from pgxcnode.c to poolmgr.c
Tomas Vondra [Sat, 4 Nov 2017 14:52:13 +0000 (15:52 +0100)]
Move several functions from pgxcnode.c to poolmgr.c

A number of functions were defined in pgxcnode.h/pgxnnode.h, but
only ever used in poolmgr.c. Those are:

- PGXCNodeConnect    - open libpq connection using conn. string
- PGXCNodePing       - ping node using connection string
- PGXCNodeClose      - close libpq connection
- PGXCNodeConnected  - verify connection status
- PGXCNodeConnStr    - build connection string

So move them to poolmgr.c and make them static, so that poolmgr
is the only part dealing with libpq connections directly.

7 years agoComments and cleanup in the connection pool manager
Tomas Vondra [Sun, 22 Oct 2017 13:00:06 +0000 (15:00 +0200)]
Comments and cleanup in the connection pool manager

Similarly to a39b06b0c6, this does minor cleanup in the pool manager
code by removing unused functions and adding a lot of comments, both
at the file level (explaining the concepts and basic API methods)
and for individual functions.

7 years agoAdd gtm_snap.c comments (missing in a39b06b0c6)
Tomas Vondra [Sat, 4 Nov 2017 16:13:23 +0000 (17:13 +0100)]
Add gtm_snap.c comments (missing in a39b06b0c6)

These comments should have been included in a39b06b0c6, but I failed
to include the file in the commit before pushing :-(

7 years agoImprove comments in GTM code, minor naming tweaks
Tomas Vondra [Sat, 4 Nov 2017 15:49:11 +0000 (16:49 +0100)]
Improve comments in GTM code, minor naming tweaks

This patch improves comments in gtm_txn.c and gtm_snap.c in three
basic ways:

1) Adds global comments explaining the basics of transaction and
   snapshot management APIs - underlying concepts, main methods.

2) Improves (and adds) function-level comments, explaining the
   meaning of parameters, return values, and other details.

3) Tweaks the naming of several API functions, to make them more
   consistent with the rest of the module.

7 years agoCleanup GTM API: make functions static, remove dead code
Tomas Vondra [Sat, 4 Nov 2017 15:46:56 +0000 (16:46 +0100)]
Cleanup GTM API: make functions static, remove dead code

The cleanup does two basic things:

* Functions used only in a single source file are made static (and also
  removed from the header file, of course). This reduces the size of the
  public GTM API.

* Unused functions (identified by the compiler thanks to making other
  functions static in the previous step) are removed. The assumption is
  that this code was not really tested at all, and would only make
  future improvements harder.

7 years agoRemove references to issue 3520503 from privileges test
Tomas Vondra [Wed, 18 Oct 2017 21:39:54 +0000 (23:39 +0200)]
Remove references to issue 3520503 from privileges test

Multiple places in the regression test mentioned issue 3520503 a reason
for failures. Unfortunately it's not clear what the issue is about (the
comments were added in 10cf12dc51), but the reference seems obsolete
anyway as the queries seem to work fine - the results are the same as
expected on upstream.

7 years agoRemove unnecessary ORDER BY from privileges test
Tomas Vondra [Wed, 18 Oct 2017 21:38:28 +0000 (23:38 +0200)]
Remove unnecessary ORDER BY from privileges test

Some of the ORDER BY clauses added to the test are no longer necessary
as the queries produce stable results anyway (all rows are the same).
So remove the unnecessary clauses, to make the test more like upstream.

7 years agoIncrease random_page_cost to fix a plan in updatable_views
Tomas Vondra [Wed, 18 Oct 2017 20:23:11 +0000 (22:23 +0200)]
Increase random_page_cost to fix a plan in updatable_views

The remote part of a query happens with per-node statistics, i.e. with
only a fraction of the total number of rows. This affects the costing
and may result in somewhat unexpected plan changes.

For example one of the plans in updatable_views changed from hashjoin
to nestloop due to this - the index got a bit smaller, lowering the
cost of inner index scan enough to make nestloop cheaper.

Instead of increasing the number of rows in the test to make it more
expensive again (which would affect the rest of the test), tweak the
random_page_cost for that one query a bit.

7 years agoCollect index statistics during ANALYZE on coordinator
Tomas Vondra [Wed, 18 Oct 2017 20:22:32 +0000 (22:22 +0200)]
Collect index statistics during ANALYZE on coordinator

ANALYZE was not collecting index statistics, which may have negative
impact for example on selectivity estimates for expressions. This also
fixes some incorrect plan changes in updatable_views regression test.

Discussion: <c822a7ff-7c53-ebaf-6f34-03132cd27621@2ndquadrant.com>

7 years agoFix failures in updatable_views due to different structure
Tomas Vondra [Wed, 18 Oct 2017 14:55:40 +0000 (16:55 +0200)]
Fix failures in updatable_views due to different structure

Since commit 93cbab90b0c6fc3fc4aa515b93057127c0ee8a1b we enforce
stricter rules on structure of partitioned tables, e.g. we do not
allow different order of columns in parent/child tables.

This was causing failures in the updatable_views tests, so fix that
by ensuring the structure actually matches exactly.

7 years agoFix handling of root->distribution during redistribution
Tomas Vondra [Thu, 5 Oct 2017 15:21:43 +0000 (17:21 +0200)]
Fix handling of root->distribution during redistribution

This fixes some remaining bugs in handling root->distribution, caused
by the upper-planner pathification (in PostgreSQL 9.6).

Prior to the pathification (so in PostgreSQL 9.5 and Postgres-XL 9.5),
the root->distribution was used for two purposes:

* To track distribution expected by ModifyTable (UPDATE,DELETE), so
  that grouping_planner() knew how to redistribute the data.

* To communicate the resulting distribution from grouping_planner()
  back to standard_planner().

This worked fine in 9.5 as grouping_planner() was only dealing with
a single remaining path (plan) when considering the redistribution,
and so it was OK to tweak root->distribution.

But since the pathification in 9.6 that is no longer true. There is
no obvious reason why all the paths would have to share the same
distribution, and we don't know which one will be the cheapest one.

So from now on root->distribution is used to track the distribution
expected by ModifyTable. Distribution for each path is available in
path->distribution if needed.

Note: We still use subroot->distribution to pass information about
distribution of subqueries, though. But we only set it after the
one cheapest path is selected.

7 years agoAccept plan changes in updatable_views test
Tomas Vondra [Tue, 17 Oct 2017 20:01:27 +0000 (22:01 +0200)]
Accept plan changes in updatable_views test

After getting rid of the extra targetlist entries in 2d29155679, the
plan changes in updatable_views seem reasonable so accept them.

7 years agoRemove coordinator quals, evaluated at Remote Subquery
Tomas Vondra [Tue, 17 Oct 2017 18:12:49 +0000 (20:12 +0200)]
Remove coordinator quals, evaluated at Remote Subquery

While rewriting UPDATE/DELETE commands in rewriteTargetListUD, we've
been pulling all Vars from quals, and adding them to target lists. As
multiple Vars may reference the same column, this sometimes produced
plans with duplicate targetlist entries like this one:

  Update on public.t111
    -> Index Scan using t1_a_idx on public.t1
       Output: 100, t1.b, t1.c, t1.a, t1.a, t1.a, t1.a, t1.a, t1.a,
               t1.a, t1.a, t1.ctid
    -> ...

Getting rid of the duplicate entries would be simple - before adding
entry for eachh Vars, check that a matching entry does not exist yet.
The question however is if we actually need any of this.

The comment in rewriteTargetListUD() claims we need to add the Vars
because of "coordinator quals" - which is not really defined anywhere,
but it probably means quals evaluated at the Remote Subquery node.
But we push all quals to the remote node, so there should not be any
cases where a qual would have to be evaluated locally (or where that
would be preferable).

So just remove all the relevant code from rewriteHandler.c, which
means we produce this plan instead:

  Update on public.t111
    -> Index Scan using t1_a_idx on public.t1
       Output: 100, t1.b, t1.c, t1.ctid
    -> ...

This affects a number of plans in regression tests, but the changes
seem fine - we simply remove unnecessary target list entries.

I've also added an assert to EXPLAIN enforcing the "no quals" rule
for Remote Subquery nodes.

Discussion: <95e80368-1549-a921-c5e2-7e0ad9485bd3@2ndquadrant.com>

7 years agoAllocate thread-local snapshot array statically
Tomas Vondra [Sat, 14 Oct 2017 10:38:06 +0000 (12:38 +0200)]
Allocate thread-local snapshot array statically

Since commit fb56418d66 the snapshots are computed in thread-local
storage, but we haven't been freeing the memory (on thread exit).
As the memory is allocated in the global (TopMostMemoryContext),
this presented a memory leak of 64kB on each GTM connection.

One way to fix this would be to track when the thread-local storage
is used in GTM_GetTransactionSnapshot(), and allocate the memory
in TopMemoryContext (which is per-thread and released on exit).

But there's a simpler way - allocate the thread-specific storage as
part of GTM_ThreadInfo, and just redirect sn_xip from the snapshot.
This way we don't have to worry about palloc/pfree at all, and we
mostly assume that every connection will need to compute at least
one snapshot anyway.

Reported by Rob Canavan <[email protected]>, investigation and fix
by me. For more discussion see
<CAFTg0q6VC_11+c=Q=gsAcDsBrDjvuGKjzNwH4Lr8vERRDn4Ycw@mail.gmail.com>

Backpatch to Postgres-XL 9.5.

7 years agoRemember queryId for queries executed using FQS
Tomas Vondra [Sat, 14 Oct 2017 10:26:13 +0000 (12:26 +0200)]
Remember queryId for queries executed using FQS

pgxc_FQS_planner() was not copying queryId, so extensions relying on
it did not work properly. For example the pg_stat_statements extension
was ignoring queries executed using FQS entirely.

Backpatch to Postgres-XL 9.5.

7 years agoRemove unnecessary ORDER BY clauses from plpgsql tests
Tomas Vondra [Sun, 8 Oct 2017 23:49:34 +0000 (01:49 +0200)]
Remove unnecessary ORDER BY clauses from plpgsql tests

Some of the tests produce stable ordering even without the explicit
ORDER BY clauses, due to only using generate_series() and not any
distributed tables. So get rid of the unnecessary ORDER BY clauses.

7 years agoFix expected output for plpgsql test suite
Tomas Vondra [Sun, 8 Oct 2017 23:30:44 +0000 (01:30 +0200)]
Fix expected output for plpgsql test suite

Commit 7d55b3a318 accepted incorrect expected output for a number
of tests in this suite. The issue might have been initially masked
by existence of another .out file for this test.

We seem to be producing the correct output, so just use expected
output from upstream. Moreover, the table (INT4_TBL) is defined as
replicated, so we don't need the explicit ORDER BY clauses as the
ordering is stable anyway. So remove them, to make the the tests
a bit closer to upstream.

7 years agoAccept correct output/plan in subselect test suite
Tomas Vondra [Sun, 8 Oct 2017 20:14:16 +0000 (22:14 +0200)]
Accept correct output/plan in subselect test suite

The value 200 is in fact incorrect, and commit 159912518 accepted it
by mistake. The query should have produced 100 (which it now does).

The plan is correct, and matches the plan produced on PostgreSQL 10
(although with Remote Subquery Scan on top).

7 years agoUpdate cmin values in combocid based on XL 9.5
Tomas Vondra [Sun, 8 Oct 2017 22:01:26 +0000 (00:01 +0200)]
Update cmin values in combocid based on XL 9.5

As mentioned in 3a64cfdde3, some of the output differences (compared
to PostgreSQL 10) may be caused by XL advancing cmin more often, for
example due to splitting a single command into multiple steps.

So tweak the expected output using output from Postgres-XL 9.5r1.6.

7 years agoStabilize combocid test by replicating the table
Tomas Vondra [Sun, 8 Oct 2017 21:44:27 +0000 (23:44 +0200)]
Stabilize combocid test by replicating the table

Commit 1d14325822 randomized the choice of a starting node with
ROUNDROBIN distribution, so that the data in combocid tests are not
all placed on the first node but distributed randomly (despite using
single-row INSERTS as before).

So to stabilize the test, make the table replicated. The table only
has a single column and the test updates is, so we can't use any
other stable distribution (e.g. BY HASH).

The expected results were obtained by running the combocid.sql on
PostgreSQL 10, so there might be some cmin differences.

7 years agoAccept warnings about inheriting distribution from parent
Tomas Vondra [Sun, 8 Oct 2017 18:36:35 +0000 (20:36 +0200)]
Accept warnings about inheriting distribution from parent

Commit e26a0e07d8 started ignoring distributions defined on partitions,
but omitted this place in 'rules' when accepting the warnings.

7 years agoAdd explicit VACUUM to stabilize plans in inherit tests
Tomas Vondra [Sun, 8 Oct 2017 18:16:47 +0000 (20:16 +0200)]
Add explicit VACUUM to stabilize plans in inherit tests

On stock PostgreSQL, CREATE INDEX also updates statistics in pg_class
(relpages and reltuples). But Postgres-XL does not do that, which may
result in plan differences when the test relies on this behavior.

This is the same issue as in cfb055553687c257dd1d1ed123356c892f48a804,
but affecting inherit regression tests. So fix it in the same way, by
doing an explicit vacuum on the tables.

7 years agoAccept plan changes in 'limit' test suite
Tomas Vondra [Sun, 8 Oct 2017 17:28:41 +0000 (19:28 +0200)]
Accept plan changes in 'limit' test suite

The accepted plan changes seem correct, as the only difference with
respect to upstream plans is Limit distribution. The commit diff is
a bit more complicated, because the expected plan did not reflect
the switch from Result to ProjectSet.

7 years agoAccept plan change in 'inherit' test suite
Tomas Vondra [Sun, 8 Oct 2017 17:22:36 +0000 (19:22 +0200)]
Accept plan change in 'inherit' test suite

The plan change is expected, as it simply expands

    -> Limit

to
    -> Limit
        -> Remote Subquery Scan
            -> Limit

It wasn't accepted before probably because it was hidden by other
failures in the test suite.

7 years agoDisable FQS for cursors defined with SCROLL
Tomas Vondra [Thu, 5 Oct 2017 21:05:27 +0000 (23:05 +0200)]
Disable FQS for cursors defined with SCROLL

When checking if a query is eligible for FQS (fast-query shipping),
disable the optimization for queries in SCROLL cursors, as FQS does
not support backward scans.

Discussion: <e66932f3-3c35-cab0-af7e-60e8dfa423ba@2ndquadrant.com>

7 years agoFix the pg_basebackup call when adding standby nodes
Tomas Vondra [Sat, 30 Sep 2017 16:58:53 +0000 (18:58 +0200)]
Fix the pg_basebackup call when adding standby nodes

When adding standby nodes using pgxc_ctl, it's calling pg_basebackup
internally. But the "-x" option was removed in PostgreSQL 10, so the
call is failing.

A straightforward fix would be to use "-X fetch" which does exactly
what "-x" used to do. But I've decided to use "-X stream" instead,
as that does not rely on wal_keep_segments.

Reported by Tank.zhang"<6220104@qq.com>, along with "-X fetch" fix.

7 years agoFix minor regression failure in updatable_views
Tomas Vondra [Tue, 26 Sep 2017 20:13:20 +0000 (22:13 +0200)]
Fix minor regression failure in updatable_views

The ATTACH PARTITION command was failing due to mismatching column
order. That in turn caused failure in sanity_check, as the table
was not dropped when dropping the parent.

7 years agoFix expected output in select_view test
Tomas Vondra [Tue, 26 Sep 2017 19:34:28 +0000 (21:34 +0200)]
Fix expected output in select_view test

The expected output did not match, likely due to some confusion when
merging upstream changes (where the query does not include the ORDER
BY clause).

The updated output matches exactly to what is produced by upstream
after adding the ORDER BY clause.

7 years agoImprove shared queue synchronization further
Pavan Deolasee [Wed, 20 Sep 2017 10:07:56 +0000 (15:37 +0530)]
Improve shared queue synchronization further

Our efforts to improve shared queue synchronization continues. We now have a
per queue producer lwlock that must be held for synchronization between
consumers and the producer. Consumers must hold this lock before setting the
producer latch to ensure the producer does not miss out any signals and does
not go into unnecessary waits.

We still can't get rid of all the timeouts, especially we see that sometimes a
producer finishes and tries to unbind from the queue, even before a consumer
gets chance to connect to the queue. We left the 10s wait to allow consumers to
connect. There is still net improvement because when the consumer is not going
to connect, it tells the producer and we avoid the 10s timeout, like we used to
see earlier.

7 years agoEnable Hot Standby on the replicas
Pavan Deolasee [Wed, 20 Sep 2017 09:30:04 +0000 (15:00 +0530)]
Enable Hot Standby on the replicas

We had an issue with tracking knownXids on the standby and it was overflowing
the allocated array in the shared memory. It turned out that the primary reason
for this is that the GTM leaves behind a hole in XID allocation when it's
restarted. The standby oblivious to this, was complaining about array overflow
and thus die.

We now fix this by allocating array which can hold CONTROL_INTERVAL worth
additional XIDs. This would mostly be a waste because the XIDs are never
allocated. But this seems like a quick fix to further test the Hot standby. The
good thing is that we might just waste memory, but not have any impact on the
performance because of larger array since we only loop for numKnownXids which
will be more accurate.

With this change, also fix the defaults for datanode and coordinator standbys
and make them Hot Standbys. The wal_level is changed too.

7 years agoHandle Aggref->aggargtypes in out/readfuncs.c
Tomas Vondra [Tue, 19 Sep 2017 21:06:46 +0000 (23:06 +0200)]
Handle Aggref->aggargtypes in out/readfuncs.c

When communicating with other nodes, we send names of objects instead
of OIDs as those are assigned on each node independently. We failed to
do this for Aggref->aggargtypes, which worked fine for built-in data
types (those have the same OID on all nodes), but resulted in failures
for custom data types (like for example FIXEDDECIMAL).

   ERROR:  cache lookup failed for type 16731

This fixes it by implementing READ/WRITE_TYPID_LIST_FIELD, similarly
to what we had for RELID.

Note: Turns out the WRITE_RELID_LIST_FIELD was broken, but apparently
we never call it in XL as it's only used for arbiterIndexes field. So
fix that too, in case we enable the feature in the future.

7 years agoEnsure that we don't read rule definition with portable input on
Pavan Deolasee [Mon, 18 Sep 2017 07:19:17 +0000 (12:49 +0530)]
Ensure that we don't read rule definition with portable input on

Rules are converted in their string representation and stored in the catalog.
While building relation descriptor, this information is read back and converted
into a Node representation. Since relation descriptors could be built when we
are reading plan information sent by the remote server in a stringified
representation, trying to read the rules with portable input on may lead to
unpleasant behaviour. So we must first reset portable input and restore it back
after reading the rules. The same applies to RLS policies (even though we don't
have a test showing the impact, but it looks like a sane thing to fix anyways)

7 years agoFix trivial failure in rangefuncs test suite
Tomas Vondra [Sun, 17 Sep 2017 19:03:34 +0000 (21:03 +0200)]
Fix trivial failure in rangefuncs test suite

Commit f7d1d581c950191a465b8483173f2ad69ae8fffe converted a couple of
sequences to persistent (instead of temporary), but failed to update
the expected output.

7 years agoFix incorrect planning of grouping sets
Tomas Vondra [Fri, 15 Sep 2017 00:13:37 +0000 (02:13 +0200)]
Fix incorrect planning of grouping sets

Commit 04f96689945462a4212047f03eb3281fb56bcf2f incorrectly allowed
distributed grouping paths for grouping sets, causing failures in
'groupingsets' regression test suite. So fix that by making sure
try_distributed_aggregation=false for plans with grouping sets.

7 years agoEnsure that database objects are created consistently.
Pavan Deolasee [Tue, 12 Sep 2017 06:47:19 +0000 (12:17 +0530)]
Ensure that database objects are created consistently.

We now create views/materialised views on all nodes, unless they are temporary
objects in which case they are created only on the local coordinator and the
datanodes. Similarly, temporary sequences are created on the local coordinator
and the datanodes.

This solves many outstanding problems in the regression results where remote
nodes used to fail because of non-existent type for a view or similar such
issues. A few other test cases now started to work correctly and produce output
matching upstream PG. So the expected output for those test cases has been
appropriated fixed.

Couple of sequences in the rangefuncs test case have been converted into
permanent sequences because the subsequent SQL functions refer to them and
hence fail if they do not exist on the remote coordinators.

The problem with special RULE converting a regular table into a view goes away
with the fix since DROP VIEW commands are now propgataed to the datanodes too.

7 years agoFurther refactoring of utility.c code
Pavan Deolasee [Mon, 11 Sep 2017 09:47:15 +0000 (15:17 +0530)]
Further refactoring of utility.c code

Furthre more simplification and consolidation of the code.

7 years agoRearrange switch cases so that they are grouped together when possible
Pavan Deolasee [Fri, 8 Sep 2017 07:06:20 +0000 (12:36 +0530)]
Rearrange switch cases so that they are grouped together when possible

7 years agoRefactor changes in the utility.c
Pavan Deolasee [Fri, 8 Sep 2017 06:28:31 +0000 (11:58 +0530)]
Refactor changes in the utility.c

7 years agoStamp Postgres-XL 10alpha2
Pavan Deolasee [Thu, 7 Sep 2017 05:00:26 +0000 (10:30 +0530)]
Stamp Postgres-XL 10alpha2

7 years agoAssorted fixes to documentation compilation
Pavan Deolasee [Thu, 7 Sep 2017 05:27:51 +0000 (10:57 +0530)]
Assorted fixes to documentation compilation

7 years agoAccept differences in tsm_system_time contrib module
Tomas Vondra [Wed, 30 Aug 2017 23:33:26 +0000 (01:33 +0200)]
Accept differences in tsm_system_time contrib module

Trivial plan changes and missing bits (likely due to incorrect merge
or something like that).

7 years agoAccept plan changes in tsm_system_rows contrib module
Tomas Vondra [Wed, 30 Aug 2017 23:31:55 +0000 (01:31 +0200)]
Accept plan changes in tsm_system_rows contrib module

Trivial changes due to distributing the queries.

7 years agoStabilize ordering in tablefunc contrib module
Tomas Vondra [Wed, 30 Aug 2017 23:23:27 +0000 (01:23 +0200)]
Stabilize ordering in tablefunc contrib module

Add explicit ORDER BY clause to stabilize ordering of test results.

7 years agoRemove FDW objects from tests in pgstattuple contrib module
Tomas Vondra [Wed, 30 Aug 2017 23:22:13 +0000 (01:22 +0200)]
Remove FDW objects from tests in pgstattuple contrib module

Postgres-XL does not support FWD object, so the tests were failing
with "does not exist" errors, instead of testing that visibility on
FDW objects is not supported. So just remove the few test queries.

7 years agoRemove FDW objects from tests in pg_visibility contrib module
Tomas Vondra [Wed, 30 Aug 2017 23:18:00 +0000 (01:18 +0200)]
Remove FDW objects from tests in pg_visibility contrib module

Postgres-XL does not support FWD object, so the tests were failing
with "does not exist" errors, instead of testing that visibility on
FDW objects is not supported. So just remove the few test queries.

7 years agoStabilize ordering of results in pg_trgm contrib module
Tomas Vondra [Wed, 30 Aug 2017 23:12:35 +0000 (01:12 +0200)]
Stabilize ordering of results in pg_trgm contrib module

Add explicit ORDER BY clause to stabilize ordering of results for
a few tests. Accept a simple plan change, distributing a LIMIT query.

7 years agoAccept plan changes in btree_gist contrib module
Tomas Vondra [Wed, 30 Aug 2017 22:45:10 +0000 (00:45 +0200)]
Accept plan changes in btree_gist contrib module

The changes are fairly simple and generally expected due to distributing
upstream queries, so adding either Remote Fast Query Execution or Remote
Subquery Scan nodes.

An explicit ORDER BY was added to a few queries to stabilize the output.

7 years agoAccept plan change in btree_gin contrib module
Tomas Vondra [Wed, 30 Aug 2017 22:42:15 +0000 (00:42 +0200)]
Accept plan change in btree_gin contrib module

The upstream plan changes due to distributing to multiple nodes.

7 years agoAccept plan changes in bloom contrib module
Tomas Vondra [Wed, 30 Aug 2017 22:35:57 +0000 (00:35 +0200)]
Accept plan changes in bloom contrib module

The changes are fairly simple and generally expected due to distributing
upstream queries, so adding either Remote Fast Query Execution or Remote
Subquery Scan nodes.

7 years agoDisable logical decoding as unsupported
Tomas Vondra [Wed, 30 Aug 2017 21:55:30 +0000 (23:55 +0200)]
Disable logical decoding as unsupported

Commit 665c224a6b2afa disabled CREATE PUBLICATION/SUBSCRIPTION, but
it was still possible to create a logical replication slot and call
pg_logical_slot_get_changes() on it.

That would however crash and burn as ReorderBufferCommit() relies on
subtransactions, and BeginInternalSubTransaction() is not expected
to fail, leading to segfaults in the PG_CATCH block.

Simply disallowing creating logical slots (and whatever else relies
on CheckLogicalDecodingRequirements) seems like the best fix.

7 years agoAccept regression diffs in stats_ext test case
Pavan Deolasee [Wed, 30 Aug 2017 05:45:28 +0000 (11:15 +0530)]
Accept regression diffs in stats_ext test case

The actual result matches with the upstream result. The diffs must have been
caused by incorrect merge. So accept those fully.

7 years agoFetch the target remote nodes to run CREATE STATISTICS command
Pavan Deolasee [Wed, 30 Aug 2017 05:40:19 +0000 (11:10 +0530)]
Fetch the target remote nodes to run CREATE STATISTICS command

Some database objects are created only on a subset of nodes. For example, views
are created only on the coordinators. Similarly, temp tables are created on the
local coordinator and all datanodes. So we must consult the relation kind
before executing the CREATE STATISTICS command on the remote nodes. Otherwise
we might try to execute it on a node where the underlying object is missing,
resulting in errors.

Patch by senhu ([email protected]) which was later reworked by me.

7 years agoAccept regression diffs in update test case
Pavan Deolasee [Tue, 29 Aug 2017 11:26:28 +0000 (16:56 +0530)]
Accept regression diffs in update test case

The first diff was in fact a mistake and the actual output matched with the
upstream output. It must have been missed during the merge process. The other
diff was simply an error because XL doesn't allow updating distribution key
column.

7 years agoFix plpgsql regression test
Pavan Deolasee [Tue, 29 Aug 2017 10:24:45 +0000 (15:54 +0530)]
Fix plpgsql regression test

There were two broad categories of problems.

1. Errors due to lack of savepoint support
2. Errors and side effects due to lack of trigger support.

For 1, we reorganised the test case so that they can be run without savepoint.
For 2, we mostly accepted the regression changes. Apart from usual errors while
creating/dropping triggers, there were differences in query results because
of changes to preceding update/insert/delete statements. The behaviour of those
statements change because of lack of triggers.

7 years agoFix alter_table test case
Pavan Deolasee [Tue, 29 Aug 2017 06:05:30 +0000 (11:35 +0530)]
Fix alter_table test case

Because of XL's strict requirement on column ordering and positioning, change
the test case to avoid DROP COLUMN commands. This kinda makes the test case
useless because the sole purpose of the test was to test if things stand up
well when there is a mismatch in column numbering. Given the current
restriction, there is no other option to make these changes.

7 years agoDo not add any distribution to a dummy append node
Pavan Deolasee [Mon, 28 Aug 2017 10:03:47 +0000 (15:33 +0530)]
Do not add any distribution to a dummy append node

A dummy append node with no subpaths doesn't need any adjustment for
distribution. This allows us to actually correct handle UPDATE/DELETE in some
cases which were failing earlier.

7 years agoMuch restructing of rowsecurity test case
Pavan Deolasee [Mon, 28 Aug 2017 09:47:32 +0000 (15:17 +0530)]
Much restructing of rowsecurity test case

- Some problems related to inherited tables fixed by ensuring column ordering.
- ORDER BY clauses added at some other places to ensure consistent row ordering.
- Changes related to TABLESAMPLE accepted as XL returns more rows than PG
- SAVEPOINTs removed and replaced by transaction blocks as XL does not support
  subtransaction
- NOTICEs are not displayed in XL
- Append is pushed down to the remote node now that we impose stricter
  restrictions on inheritance

7 years agoMake float8_tbl replicated to stabilize float8 test
Tomas Vondra [Fri, 25 Aug 2017 22:05:15 +0000 (00:05 +0200)]
Make float8_tbl replicated to stabilize float8 test

As the table has just a single float8 column, XL automatically picks
ROUNDROBIN distribution. Commit 1d14325822 randomized selection of the
initial node, which with single-row inserts (used by the float8 tests)
effectively means random distribution, while before that all the rows
would be routed to the first node.

Some of the tests in float8 test suite seem to be sensitive to this, in
particular this overflow test:

    SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f ORDER BY f1;
    ERROR:  value out of range: overflow

One of the rows (containing 1.2345678901234e-200) triggers an underflow,
so when the database hits it first, it will report this error instead:

    ERROR:  value out of range: underflow

The probability of hitting this is however fairly low (less than 10%),
as the executor has to touch the underflowing value first.

7 years agoFix prepared_xacts test case
Pavan Deolasee [Fri, 25 Aug 2017 13:04:47 +0000 (18:34 +0530)]
Fix prepared_xacts test case

Remove a SAVEPOINT statement, which otherwise fails. Once that is removed, a
few other test cases work fine and the associated expected output changes are
accepted.

7 years agoAccept regression diffs in select_implicit test case
Pavan Deolasee [Fri, 25 Aug 2017 12:18:25 +0000 (17:48 +0530)]
Accept regression diffs in select_implicit test case

It was a simple case of change in row ordering because the test case is
requesting order by column 'a', but the expected output had order by column 'c'

7 years agoAccept regression diff in tablesample test case
Pavan Deolasee [Fri, 25 Aug 2017 12:11:01 +0000 (17:41 +0530)]
Accept regression diff in tablesample test case

It's just an addition of a Remote Subquery Scan node on top of the regular
plan.

7 years agoDo not try to set invalid value for guc
Pavan Deolasee [Fri, 25 Aug 2017 11:35:25 +0000 (17:05 +0530)]
Do not try to set invalid value for guc

We don't support subtransactions and hence can't handle exception thrown by
trying to set invalid value. We'd already removed the exception, but the
transaction was being left in an aborted state. So fix this.

The test case still fails for some other reason which should be investigated
separately.

7 years agoAccept regression diffs in select_views test case
Pavan Deolasee [Fri, 25 Aug 2017 11:06:45 +0000 (16:36 +0530)]
Accept regression diffs in select_views test case

These changes were lost when we removed alternate expected output files for the
test case. So these are not new differences and the same ordering is exhibited
in XL9.5 as well.

NOTICEs are not shown by XL so accept those differences.

7 years agoMake adjustment to foreign_key test case
Pavan Deolasee [Fri, 25 Aug 2017 06:39:42 +0000 (12:09 +0530)]
Make adjustment to foreign_key test case

Accept some diffs which look sane and in-line with the upstream errors. Also
comment out a few tests which explictly test subtransactions, something we
don't currently support.

7 years agoAccept errors in hash_index test case
Pavan Deolasee [Fri, 25 Aug 2017 06:09:48 +0000 (11:39 +0530)]
Accept errors in hash_index test case

We don't support BACKWARD scan of RemoteSubplan and neither support WHERE
CURRENT OF. So accept the resulting errors.

7 years agoAccept errors in tidscan regression test
Pavan Deolasee [Fri, 25 Aug 2017 05:56:38 +0000 (11:26 +0530)]
Accept errors in tidscan regression test

We don't support BACKWARD scan of RemoteSubplan and neither support WHERE
CURRENT OF. So accept errors arising out of these limitations. These test case
changes are new in XL10 and hence we did not see these failures in the earlier
releases of XL.

7 years agoMake adjustments to combocid test so that it passes
Pavan Deolasee [Thu, 24 Aug 2017 12:28:38 +0000 (17:58 +0530)]
Make adjustments to combocid test so that it passes

SAVEPOINTs were used, which we don't support. So commented those out (along
with ROLLBACK calls). That does have an impact on the test case though because
at least in one place we were checking if the cmin goes back to 0 after rolling
back to a savepoint. But there is not much we can do about that until we add
SAVEPOINT support. Other changes include accepting diffs because of ctid
changes as rows come from different nodes and hence ctids are
duplicated/changed.

7 years agoAccept regressions diffs in copy2 test case
Pavan Deolasee [Thu, 24 Aug 2017 10:12:15 +0000 (15:42 +0530)]
Accept regressions diffs in copy2 test case

We do not support trigger and hence the regression differences.