Rather than appending to a StringInfo, return a string. The caller can
append that to a StringInfo if he wants to. And instead of passing a
prefix as argument, the caller can prepend that too.

Both callers passed the same format string, so just embed that in the
function itself.

Don't append a trailing "; ". It's easier for the caller to append it,
if it's preferred, than to remove it afterwards.

Also add a regression test for the 'gp_enable_fallback_plan' GUC. There
were none before. The error message you get with that GUC disabled uses the
gp_guc_list_show function.

Plus other minor cleanup.

The locking contract to access LSN of a page is:
1. Content lock must be held in exclusive mode,
OR
2. Content lock must be held in shared mode and buffer header spinlock
must be held.

PageGetLSN() and BufferGetLSNAtomic() now assert that this contract is
maintained for shared buffers. To make the implementation for
PageGetLSN() a little easier, move to a static inline function instead
of a macro. Callers passing a PageHeader must now explicitly cast to
Page.
Signed-off-by: NAsim R P <apraveen@pivotal.io>
Signed-off-by: NAshwin Agrawal <aagrawal@pivotal.io>

Certain callers of PageGetLSN weren't correctly holding the buffer
spinlock; it is needed whenever the buffer content lock is not held in
exclusive mode.

For heapam.c, also ensure that we don't access the LSN after releasing
the lock.
Signed-off-by: NAsim R P <apraveen@pivotal.io>

'nuff said.

Original commit message:

This speeds up reassigning locks to the parent owner, when the transaction
holds a lot of locks, but only a few of them belong to the current resource
owner. This is particularly helps pg_dump when dumping a large number of
objects.

The cache can hold up to 15 locks in each resource owner. After that, the
cache is marked as overflowed, and we fall back to the old method of
scanning the whole local lock table. The tradeoff here is that the cache has
to be scanned whenever a lock is released, so if the cache is too large,
lock release becomes more expensive. 15 seems enough to cover pg_dump, and
doesn't have much impact on lock release.

Jeff Janes, reviewed by Amit Kapila and Heikki Linnakangas.

It was getting in the way of backporting commit 9b1b9446f5 from PostgreSQL,
which added an '#include "storage/lock.h"' to resowner.h, forming a cycle.

The include was only needed for the decalaration of awaitedOwner global
variable. Replace "ResourceOwner" with the equivalent "struct
ResourceOwnerData *" to avoid it.

This revealed a bunch of other files that were relying on resowner.h
being indirectly included through lock.h. Include resowner.h directly
in those files.

The ResPortalIncrement.owner field was not used for anything, so instead
of including resowner.h in that file, just remove the field that needed
it.

An FTS probe to the primaries in a mirrorless cluster will never
result in the update of gp_segment_configuration.  If a primary goes
down, we must keep the primary marked as up so that gpstart can start
the primary back up.  All transactions will abort and nothing should
work except for read-only queries to master-only catalog tables.

Stopping the FTS probes for mirrorless cluster introduced an infinite
loop in FtsNotifyProber in which the dispatcher waits in an infinite
loop for fts_statusVersion to change.  To break the infinite loop, we
acknowledge the forced probe request as a no-op and update
fts_statusVersion to break the loop for the dispatcher.  The
dispatcher should then act same as before this commit.

We also add #define for character value of GpFaultStrategy.

Authors: Xin Zhang, Ashwin Agrawal, and Jimmy Yih

Introduced by commit 522c7c09, spotted by Coverity.

Any error that the user is expected to see should be using the
ereport() macro rather than its older cousin elog(). Also avoid
closing resources just before erroring out as the error cleanup
will be handled automatically, and move a long message to have
an errhint instead.

I'm not sure if we broke this during the 8.3 merge, or if it was broken
even before that, but our version of heap_form_tuple was confused over
what the isnull argument is. The difference between heap_form_tuple()
and heap_formtuple() is the values used in the "isnull" array, but
our heap_form_tuple passed a 'char' array to the internal heap_fill_tuple()
function, which assumes a 'bool' arry.

That caused an assertion failure in hstore regression tests (and almost
certainly a crash or at least wrong results without assertions, though I
didn't check.)

To fix, take the version of heap_form_tuple() from PostgreSQL 8.4, which
converts the 'char' array to a 'bool' array, and then calls the new
heap_formtuple() function.

SPIMemReserved was initialized in CreateSharedMemoryAndSemaphores which
make no sense, this commit put it under InitResManager where the resource
memory policy is set.

As a pleasent side-effect, the window specification in a
"func_expr OVER (window_spec)" rule now gets its parse location set
correctly. Thanks to that, a few error messages now give a user-friendly
error location line.

The resource group is enabled but not initialized on auxiliary processes
and special backends like ftsprobe and filerep, previously we performed
resource group operations no matter resource group is initialized or not
which leads to some unexpected error.

Signed-off-by: NXiaoran Wang <xiwang@pivotal.io>
Signed-off-by: NAdam Lee <ali@pivotal.io>

Use ereport(), with a proper error code, for errors that are "expected" to
happen sometimes, like missing configuration files, or network failure.

That's nicer in general, but the reason I bumped into this is that internal
error messages include a source file name and line number in GPDB, and if
those error messages are included in the expected output of regression
tests, those tests will fail any time you do change the file, so that the
elog() moves around and the line number changes.

An assertion with a side effect may alter the main codepath when
the tree is built with --enable-cassert, which in turn may lead
to subtle differences due compiler optimizations and/or straight
bugs in the side effect. Rewrite the assertions without side
effects to leave the main codepath intact.

We used to use sequence scan on pg_resgroupcapability for functions that
need to do a full scan of pg_resgroupcapability. Problem is accessing
in this way will take a long time after pg_resgroupcapability table was
updated/deleted million times as our stress tests do, pg_resgroupcapability
was filled of invalid blocks and sequence scan wasted lots of time to bypass
those blocks. Using index scan on it can resolve this problem.

Don't send nonsense '\n' characters just for counting, let segments
report how many rows are processed instead.
Signed-off-by: NMing LI <mli@apache.org>

GUC value is `gp_enable_segment_copy_checking`, its default value is true.
User can disable the distribution key check with with a GUC value.
Signed-off-by: NXiaoran Wang <xiwang@pivotal.io>

When use command `COPY FROM ON SEGMENT`, we copy data from
local file to the table on the segment directly. When copying
data, we need to apply the distribution policy on the record to compute
the target segment. If the target segment ID isn't equal to
current segment ID, we will report error to keep the distribution
key restriction.

Because the segment has no meta data info about table distribution policy and
partition policy,we copy the distribution policy of main table from
master to segment in the query plan. When the parent table and
partitioned sub table has different distribution policy, it is difficult
to check all the distribution key restriction in all sub tables. In this
case , we will report error.

In case of the partitioned table's distribution policy is
RANDOMLY and different from the parent table, user can use GUC value
`gp_enable_segment_copy_checking` to disable this check.

Check the distribution key restriction as follows:

1) Table isn't partioned:
    Compute the data target segment.If the data doesn't belong the
    segment, will report error.

2) Table is partitioned and the distribution policy of partitioned table
as same as the main table:
    Compute the data target segment.If the data doesn't belong
    the segment, will report error.

3) Table is partitioned and the distribution policy of partitioned
table is different from main table:
    Not support to check ,report error.
Signed-off-by: NXiaoran Wang <xiwang@pivotal.io>
Signed-off-by: NMing LI <mli@apache.org>
Signed-off-by: NAdam Lee <ali@pivotal.io>

This allows setting one of the GPDB-added attributes on a line, without
modifying the original line. This reduces the diff of pg_proc.h from
upstream.

This has no effect on the resulting BKI file, except for whitespace. IOW,
there are no catalog changes in this commit. I checked that by diffing the
resulting BKI file, before and after this patch, with "diff -w".

I still left the TODO comment in pg_proc.h in place, which pointed out
that it'd be nice if we could automatically use prodataaccess = 'c' as the
default for SQL-language columns, and 'n' for others. I actually wrote a
more flexible prototype at first that could do that. In that prototype, you
could provide an arbitrary perl expression that was evaluated on every row,
and could compute a value based on other columns. But that was more
complicated, and at the same time, not as flexible, because you could still
not specify particular values for just one row. So I think this is better
in the end.

Also, I noticed that we haven't actually marked all SQL-language functions
with prodataaccess = 'c'. Tsk tsk. It's too late for catalog changes, so
not fixing that now. At some point, we should discuss whether we should
do something different with prodataaccess, like change the code so that
it's ignored for SQL language functions altogether. Or perhaps just remove
the column, the only useful value for it is the magic 's' value, which
can only be used in built-in functions because there's no DDL syntax for
it. But that's a whole different story.

It's harmless, as the genbki script ignores them, but seems a bit untidy.
It also makes it harder to diff between the re-constructed DATA lines and
the originals.

Calling elog(ERROR ..) will exit the current context, so the
cleanup function would never run. Shift around to ensure the
cleanup is called. This codepath was recently introduced in
commit 00ce2c14 where a surrounding try/catch block was removed.

Also change to ereport from elog since this is an error that
a user could run into.

As part of the 8.3 merge, we backported the addition of the location field.
from 8.4, but accidentally left the copy support for it commented out.

Also, be more pedantic about using COPY/READ/WRITE_LOCATION_FIELD macros
to deal with location fields, rather than the *_INT_FIELD macros. This is
purely cosmetic, but hopefully reduces merge conflicts a bit. I didn't touch
the few places where we already had the location field in 8.3, those will
be changed as part of the 8.4 merge.

The winlevelsup field isn't used. The reason it's not needed can be summed
up by this comment in PostgreSQL 8.4's transformWindowFunc function:

>  * Unlike aggregates, only the most closely nested pstate level need be
>  * considered --- there are no "outer window functions" per SQL spec.

Second line of reasoning is that the winlevelsup field was always
initialized to 0, and only incremented in the IncrementVarSublevelsUp
function. But that function is only used during planning, so winlevelsup
was always 0 in the parse and parse analysis stage. However, the field was
read only in the parse analysis phase, which means that it was always 0
when it was read.

Third line of reasoning is that the regression tests are happy without it,
and there was a check in the ORCA translator too, that would've thrown
an error if it was ever non-zero.

I left the field in place in the struct, to avoid a catalog change, but it
is now unsued. WindowRef nodes can be stored in catalogs, as part of views,
I believe.

I don't understand why these were modified in GPDB in the first place.
I dug into the old git history, from before Greenplum was open sourced, and
traced the change to a massive commit from 2011, which added support for
(non-recursive) WITH clause. I think the change was just collateral damage
in that patch; I don't see any relationship between WITH clause support and
these error messages.

These errors can be reproduced with queries like this:

    (select 'foobar' order by 1) order by 1;
    (select 'foobar' limit 1) limit 2;
    (select 'foobar' offset 1) offset 2;

ereport() has one subtle but important difference to elog: it doesn't
evaluate its arguments, if the log level says that the message doesn't
need to be printed. This makes a small but measurable difference in
performance, if the arguments contain more complicated expressions, like
function calls.

While performance testing a workload with very short queries, I saw some
CPU time being used in DtxContextToString. Those calls were coming from the
arguments to elog() statements, and the result was always thrown away,
because the log level was not high enough to actually log anything. Turn
those elog()s into ereport()s, for speed.

The problematic case here was a few elogs containing DtxContextToString
calls, in hot codepaths, but I changed a few surrounding ones too, for
consistency.

Simplify the mock test, to not bother mocking elog(), while we're at it.
The real elog/ereport work just fine in the mock environment.

In single-user mode, MyQueueId isn't set. But there was an assertion for
that in ResourceQueueGetQueryMemoryLimit. To fix, don't apply memory limits
in single-user mode.

In previous code, when user decreases resgroup concurrency_limit
to a value that less than the number of current running jobs in
that resgroup, the calculation of memory that need to return to
SYSPOOL is not correct and it might cause assert fail.

We add code that takes into account this situation. And the logic
here is that when we decide to return some memory to SYSPOOL, we
only return `Min(total_memory_should_return, max_mem_can_return)`.

And since alter-memory command has gradually-effect semantic, when
a job is just before ending and it finds out that its ending could
provide free slot for others(not blocked by concurrency_limit), it
will try not to return all the memory it could but reserve some to
make sure that new job would not be blocked because of memory_quota.
Signed-off-by: NGang Xiong <gxiong@pivotal.io>

We had replaced the upstream code in readfuncs.c that checks what kind of
a Node we're reading, with a seemingly smarter binary search. However, that's
a premature optimization. Firstly, the linear search is pretty darn fast,
because after compiler optimizations, it will check for the string length
first. Secondly, the binary search implementation required an extra
palloc+pfree, which is expensive enough that it almost surely destroys any
performance gain from using a binary search. Thirdly, this isn't a very
performance-sensitive codepath anyway. This is used e.g. to read view
definitions from the catalog, which doesn't happen very often. The
serialization code used when dispatching a query from QD to QEs is a more
hot codepath, but that path uses the different method, in readfast.c.

So, revert the code the way it is in the upstream. This hopefully reduces
merge conflicts in the future.

Also, there was in fact a silly bug in the old implementation. It used
wrong identifier string for the RowCompare expression. Because of that, if
you tried to use a row comparison in a view, you got an error. Fix that,
and also add a regression test for it.

The assertion didn't take into account that malloc() might return NULL.

When the new OID-dispatching mechanism was introduced (commit f9016da2),
a lot of FIXME comments were left in calls to CdbDispatchUtilityStatement.
These were places where I wasn't sure if the command that was executed
might create new objects, with new OIDs assigned to them, that would need
to be dispatched to the QE. I've now skimmed through the call sites, and
checked that they are all for ALTER or DROP commands that should not create
new OIDs, so remove the FIXME comments.

In theory, there could be an extension out there that uses it, but I doubt
it. And if there is, the fix is simple: just use the regular PG_MODULE_MAGIC
macro instead.

The duplicate was introduced by the 8.3 merge, because we had already
cherry-picked this in commit d481bd72. Harmless, but let's be tidy.

I think this is some kind of a debugging function, but since it's not
exposed in pg_proc.h, there's no easy way to use it. Seems better to just
remove it, than try to expose it.