1. 27 12月, 2018 3 次提交
    • K
      Update continuous-integration to gp-continuous-integration · 8003b1d6
      Karen Huddleston 提交于
      The repo name was changed after the github migration. The previous paths
      will not work correctly for new clones of the repository.
      Authored-by: NKaren Huddleston <khuddleston@pivotal.io>
      8003b1d6
    • N
      Ignore LockAcquire() return value in gp_expand_lock_catalog() · fb7c92d0
      Ning Yu 提交于
      Here is the return values of `LockAcquire()`:
      
          * LOCKACQUIRE_NOT_AVAIL       lock not available, and dontWait=true
          * LOCKACQUIRE_OK              lock successfully acquired
          * LOCKACQUIRE_ALREADY_HELD    incremented count for lock already held
      
      In our case `dontWait` is false so `LOCKACQUIRE_NOT_AVAIL` is never
      returned, the other 2 both indicate that the lock is successfully
      acquired.  So simply ignore the return value.
      
      This issue is detected by Coverity, here is the original report:
      
          *** CID 190295:  Error handling issues  (CHECKED_RETURN)
          /tmp/build/0e1b53a0/gpdb_src/src/backend/utils/misc/gpexpand.c: 98 in gp_expand_lock_catalog()
          92      *
          93      * This should only be called by gpexpand.
          94      */
          95     Datum
          96     gp_expand_lock_catalog(PG_FUNCTION_ARGS)
          97     {
          >>>     CID 190295:  Error handling issues  (CHECKED_RETURN)
          >>>     Calling "LockAcquire" without checking return value (as is done elsewhere 14 out of 16 times).
          98      LockAcquire(&gp_expand_locktag, AccessExclusiveLock, false, false);
          99
          100             PG_RETURN_VOID();
          101     }
          102
          103     /*
      fb7c92d0
    • A
      Bump ORCA version to 3.18.0 · 1f3cbd73
      Abhijit Subramanya 提交于
      1f3cbd73
  2. 26 12月, 2018 2 次提交
  3. 25 12月, 2018 2 次提交
    • N
      Fix expression type in FAST_PATH_SET_HOLD_TILL_END_XACT() · 4ea0e419
      Ning Yu 提交于
      In macro `FAST_PATH_SET_HOLD_TILL_END_XACT()` we expect an expression to
      return uint64 type, however the actual return type depends is decided by
      the argument `bits`, when `bits` is 32-bit wide it might not produce
      correct result.
      
      Fixed by adding necessary type conversion in the macro.
      
      This issue is detected by Coverity, here is the original report:
      
          *** CID 190294:  Integer handling issues  (BAD_SHIFT)
          /tmp/build/0e1b53a0/gpdb_src/src/backend/storage/lmgr/lock.c: 4557 in setFPHoldTillEndXact()
          4551
          4552                    if (proc->fpRelId[f] != relid ||
          4553                            (lockbits = FAST_PATH_GET_BITS(proc, f)) == 0)
          4554                            continue;
          4555
          4556                    /* one relid only occupies one slot. */
          >>>     CID 190294:  Integer handling issues  (BAD_SHIFT)
          >>>     In expression "(lockbits & 7U) << 3U * f", left shifting by more than 31 bits has undefined behavior.  The shift amount, "3U * f", is as much as 45.
          4557                    FAST_PATH_SET_HOLD_TILL_END_XACT(proc, f, lockbits);
          4558                    result = true;
          4559                    break;
          4560            }
          4561
          4562            LWLockRelease(proc->backendLock);
          4563
          4564            return result;
      4ea0e419
    • D
      Make SendTuple do de-toasting for heap as well as memtuples (#6527) · bf5e3d5d
      David Kimura 提交于
      * Make SendTuple do de-toasting for heap as well as memtuples
      
      Motion node didn't consistently handle where MemTuples and HeapTuples
      are de-toasted. This change delays de-toasting of MemTuple in motion
      sender until SerializeTupleDirect(), which is where HeapTuples are
      de-toasted.
      
      This change also corrects a bug in de-toasting of memtuples in motion
      sender.  De-toasting of memtuples marked the slot containing the
      de-toasted memtuple as virtual.  When a TupleTableSlot is marked
      virtual, its values array should point to addresses within the memtuple.
      The bug was that the values array was populated *before* de-toasting and
      the memory used by the toasted memtuple was freed.  Subsequent usages of
      this slot suffered from dereferencing invalid memory through the
      pointers in values array.
      
      This change corrects the issue by simplifying ExecFetchSlotMemTuple() to
      no longer manage memory required to de-toast which and also aligns the
      code structure more similar to ExecFetchSlotMinimalTuple() in Postgres.
      An alternative solution that we considered is to continue de-toasting in
      ExecFetchSlotMemTuple() and clear the virtual bit before freeing the
      tuple. A disadvantage of that approach is that it modifies an existing
      slot and complicates the contract of memtuple and virtual table slots.
      Also, it doesn't match with where HeapTuples are de-toasted so the
      contract becomes less clear.
      
      The bug is hard to discover because motion node in executor is the only place
      where a memtuple is de-toasted, before sending it over interconnect.  Only a
      few plan nodes make use of a tuple that's already returned to the parent nodes
      e.g. Unique (DISTINCT ON()) and SetOp (INTERSECT / EXCEPT).  Example plan that
      manifests the bug:
      
       Gather Motion 3:1
         Merge Key: a, b
         ->  Unique
               Group By: a, b
               ->  Sort
                     Sort Key (Distinct): a, b
                     ->  Seq Scan
      
      The Unique node uses the previous tuple de-toasted by the Gather Motion sender
      to compare with the new tuple obtained from the Sort node for eliminating
      duplicates.
      Co-authored-by: NAsim R P <apraveen@pivotal.io>
      bf5e3d5d
  4. 24 12月, 2018 2 次提交
  5. 23 12月, 2018 1 次提交
    • D
      Remove support for parquet relstorage · 6c0a8f61
      Daniel Gustafsson 提交于
      The parquet relstorage support was, according to the closed ticket
      system, added a long time ago in order to reach code parity with
      WAWQ. There was never a way to create a relation with relstorage 'p'
      and indeed PARQUET external tables in Greenplum use relstorage 'x'
      for external. Having dead code around as a compatibility shim with
      another database seems quite pointless, and if ORCA actually ever
      calls this then that should be fixed rather than having a lookup
      which will always fail.
      Reviewed-by: NAshwin Agrawal <aagrawal@pivotal.io>
      Reviewed-by: NBhuvnesh Chaudhary <bchaudhary@pivotal.io>
      6c0a8f61
  6. 22 12月, 2018 4 次提交
    • Z
      Fix bitmap index bug. (#6541) · cbf622de
      Zhenghua Lyu 提交于
      If bmgetbimap cannot acquire a bitmap, it return
      an empty TIDBITMAP. This will cause an error in
      the next time loop in MultiExecBitmapIndexScan.
      
      An example is shown here:
      ```sql
      SET enable_seqscan = OFF;
      SET enable_indexscan = ON;
      SET enable_bitmapscan = ON;
      
      create table bm_test (i int, t text);
      insert into bm_test select i % 10, (i % 10)::text  from generate_series(1, 100) i;
      create index bm_test_idx on bm_test using bitmap (i);
      select count(*) from bm_test where i=1;
      select count(*) from bm_test where i in(1, 8);
      ```
      The tuple with `i = 1` and the tuple with `i=8` are
      on different segments.
      
      When we execute the expr `i in (1,8)`, we do the
      following  work on each segment: loop the array
      `(1, 8)` to get two bitmaps and merge them using
      OR. (See function `MultiExecBitmapIndexScan`)
      
      Suppose on the segment where `i=8` is, it first tries
      to get bitmap for `i=1`. Since there is no bitmap for
      `i=1` on this segment, it will return an empty
      TIDBitmap(see function `bmgetbitmap`). Then the next
      time, we could find `i=8`'s bitmap, but its type is
      `StreamBitmap`, we cannot merge it with the previous
      TIDBitmap. That is the bug's root cause.
      
      In this commit, we fix this by returning an empty
      Streambitmap in the function bmgetbitmap.
      Co-authored-by: NShujie Zhang <shzhang@pivotal.io>
      cbf622de
    • M
      docs - remove track_count GUC for pg_stat_* and pg_statio_* views (#6479) · b401de33
      Mel Kiyama 提交于
      * docs - remove track_count GUC
      
      * docs - removed GUC track_activities
      
      will be backported to 5X_STABLE
      b401de33
    • H
      Fix quoting when dispatching SAVEPOINT commands. · effe6a93
      Heikki Linnakangas 提交于
      Fixes github issue #6345.
      Reviewed-by: NDaniel Gustafsson <dgustafsson@pivotal.io>
      effe6a93
    • T
      Remove obsolete references to pxf in CI scripts · 71fa5566
      Taylor Vesely 提交于
      Previously PXF was compiled and tested as part of the gpdb_master
      pipeline, but it has since been spun out into its own pipeline. These
      references were left behind, and appear to no longer be used.
      Co-authored-by: NJimmy Yih <jyih@pivotal.io>
      71fa5566
  7. 21 12月, 2018 9 次提交
    • L
      docs - must be a superuser to create a protocol (#6520) · afaec05d
      Lisa Owen 提交于
      afaec05d
    • H
      Fix ZSTD context leak on error. · b3448e52
      Heikki Linnakangas 提交于
      Track ZSTD handles with resource owners, so that they can be closed on
      abort.
      Reviewed-by: NIvan Leskin <leskin.in@arenadata.io>
      b3448e52
    • H
      Silence compiler warnings about overrunning sprintf buffers. · 77baffd8
      Heikki Linnakangas 提交于
      With GCC 8:
      
      fileam.c: In function ‘external_set_env_vars_ext’:
      fileam.c:2315:41: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
        sprintf(extvar->GP_LINE_DELIM_LENGTH, "%d", line_delim_len);
                                               ^~
      fileam.c:2315:40: note: directive argument in the range [-1, 2147483647]
        sprintf(extvar->GP_LINE_DELIM_LENGTH, "%d", line_delim_len);
                                              ^~~~
      fileam.c:2315:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
        sprintf(extvar->GP_LINE_DELIM_LENGTH, "%d", line_delim_len);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      For some reason, the compiler is only warning about GP_LINE_DELIM_LENGTH,
      and not the other small buffers used similarly. Perhaps because it
      considers it OK to overflow to another char[] field in a struct, but not
      over a pointer? But for consistency, fix them all.
      77baffd8
    • H
      Silence compiler warning with GCC 8 and -O3 · b64ec2ec
      Heikki Linnakangas 提交于
      heap.c: In function ‘heap_create_with_catalog’:
      heap.c:1678:3: error: ‘stdRdOptions’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
         InsertAppendOnlyEntry(relid,
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                stdRdOptions->blocksize,
                ~~~~~~~~~~~~~~~~~~~~~~~~
                safefswritesize,
                ~~~~~~~~~~~~~~~~
                stdRdOptions->compresslevel,
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                stdRdOptions->checksum,
                ~~~~~~~~~~~~~~~~~~~~~~~
                                     stdRdOptions->columnstore,
                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~
                stdRdOptions->compresstype,
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~
                InvalidOid,
                ~~~~~~~~~~~
                InvalidOid,
                ~~~~~~~~~~~
                InvalidOid,
                ~~~~~~~~~~~
                InvalidOid,
                ~~~~~~~~~~~
                InvalidOid);
                ~~~~~~~~~~~
      
      This cannot actually happen, 'stdRdOptions' is always set, when
      'appendOnlyRel' is true. It's a bit surprising that GCC apparently sees
      that with -O2 and below, but not with -O3...
      
      Fixes github issue https://github.com/greenplum-db/gpdb/issues/6121.
      b64ec2ec
    • H
      Silence compiler warning about using too small a buffer. · 7d33b5c5
      Heikki Linnakangas 提交于
      resgroup-ops-linux.c: In function ‘removeDir’:
      resgroup-ops-linux.c:652:17: warning: ‘%ld’ directive writing between 1 and 20 bytes into a region of size 16 [-Wformat-overflow=]
         sprintf(str, "%ld", pid);
                       ^~~
      resgroup-ops-linux.c:652:16: note: directive argument in the range [-9223372036854775808, 9223372036854775806]
         sprintf(str, "%ld", pid);
                      ^~~~~
      resgroup-ops-linux.c:652:3: note: ‘sprintf’ output between 2 and 21 bytes into a destination of size 16
         sprintf(str, "%ld", pid);
         ^~~~~~~~~~~~~~~~~~~~~~~~
      
      That couldn't overflow in reality, because a pid cannot be that large,
      but let's avoid the warning.
      7d33b5c5
    • H
      Silence compiler warning about using variable uninitialized. · acfa3d4f
      Heikki Linnakangas 提交于
      With GCC 8, -O1, I was getting this:
      
      In file included from ../../../src/include/postgres.h:47,
                       from resgroupcmds.c:14:
      resgroupcmds.c: In function ‘AlterResourceGroup’:
      ../../../src/include/c.h:852:4: error: ‘cpuset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
          strncpy(_dst, (src), _len); \
          ^~~~~~~
      resgroupcmds.c:374:14: note: ‘cpuset’ was declared here
        const char *cpuset;
                    ^~~~~~
      
      It's a false warning, all the code paths that lead here initialize
      'cpuset', but apparently GCC doesn't see that with -O1.
      acfa3d4f
    • A
      Use mode-specific directory name for pg_rewind tests · 67c5c812
      Asim R P 提交于
      The pg_rewind tests are run in two modes: local (copy mode) and remote
      (libpq mode).  When invoked as "make -C src/bin/pg_rewind
      installcheck", the same set of tests are run in both the modes.  Each
      mode involves initializing two postgres databases under tmp_check
      subdirectory.  The subsequent mode wipes out the tmp_check directory
      created for previous mode, resulting in lost evidence when debugging
      CI failures.  Resolve this by creating tmp_check_local and
      tmp_check_remote directories for the two modes.
      67c5c812
    • H
      Implement direct dispatch for IS NULL. · 7e96328e
      Heikki Linnakangas 提交于
      Perhaps not super interesting, but it's straightforward to implement, and
      ORCA was already doing it.
      Reviewed-by: NVenkatesh Raghavan <vraghavan@pivotal.io>
      7e96328e
    • A
      Remove XLogLocationToString*() and guc Debug_print_qd_mirroring. · 6d902a61
      Ashwin Agrawal 提交于
      Most of the code under guc `Debug_print_qd_mirroring` was deleted when special
      code for master mirroring like mmxlog and friends were deleted. Delete the
      remaining pieces and the GUC.
      
      Also, we were carrying diff against upstream with XLogLocationToString*()
      functions. Many of the variants were unused. The ones which were used can be
      easily replaced and avoid need for having static string buffer to construct the
      string and then print.
      6d902a61
  8. 20 12月, 2018 8 次提交
    • D
      Clean up TimeSliceCheck() in interrupt check path · 7148fc46
      Daniel Gustafsson 提交于
      Commit 2f628da3 optimized the check
      for interrupts codepath for the normal case. In case testutils are
      enabled we run an additional check in CFI, TimeSliceCheck(). This
      codepath was less optimized as it was running a few uninteresting
      asserts, and it was also leaking memory for configured elevels lower
      than ERROR. Fix by making the elevel GUC use a tailored enum and
      free the memory explicitly. Also perform minor cleanups along the
      way like tidying up the ereport() call and moving the macro defining
      CHECK_TIME_SLICE to after TimeSliceCheck() has been prototyped.
      
      Discussion: https://github.com/greenplum-db/gpdb/pull/6492
      Reviewed-by: Heikki Linnakangas
      7148fc46
    • H
      Change test to be closer to upstream. · 6df705db
      Heikki Linnakangas 提交于
      We have had to change this test, because it doesn't work as is in GPDB,
      because GPDB doesn't enforce exclusion constraints across segments. That's
      a bit dubious, of course, but this at least minimizes the diff in the
      test file.
      6df705db
    • H
      Remove FIXME, we don't need to do anything here. · 23a8c544
      Heikki Linnakangas 提交于
      Our window aggregate's now work the same as in upstream, and this contrib
      module's code is 100% identical to upstream too, except for this FIXME.
      I also tried running the tsearch2 regression tests. It failed because of
      some trivial-looking issues, but all the tests for the rewrite() aggregate
      worked fine.
      23a8c544
    • N
      Remove the icg_planner_centos7_random_numsegments pipeline job · 1d091eed
      Ning Yu 提交于
      Tables in this pipeline job are all created with random numsegments
      attribute, it was introduced to improve the test coverage.  However many
      tests only work correctly when the tables are all distributed on 3
      segments, so they are flaky in this job.  One example is the
      gp_dist_random('table') function, it only returns rows from segments
      0~N-1 where N is numsegments of the table.
      
      Remove this job for now, we could add it back later with a carefully
      picked subset of tests.
      1d091eed
    • P
      WalSnd struct is not well protected and manipulated. (#6493) · 30460ebd
      Paul Guo 提交于
      Its member replica_disconnected_at is not protected by lock. Now we
      protect it using walsnd->mutex, and this member is not initialized
      sometimes. These sometimes lead to an assert failure with stack as below,
      
      \#2  0x0000000000acfff4 in ExceptionalCondition (conditionName=0xdb08f0 "!(walsnd_replica_disconnected_at)", errorType=0xdb08c1 "FailedAssertion", fileName=0xdb08b0 "gp_replication.c", lineNumber=68) at assert.c:66
      \#3  0x0000000000916148 in GetMirrorStatus (response=0x7ffe81dbb620) at gp_replication.c:68
      \#4  0x00000000007a4aa9 in HandleFtsWalRepProbe () at ftsmessagehandler.c:264
      \#5  0x00000000007a4c91 in HandleFtsMessage (query_string=0x23c92f0 "PROBE") at ftsmessagehandler.c:359
      
      Besides, GetMirrorStatus() does not manipulate WalSnd under lock.
      
      Reviewed by Heikki Linnakangas and Asim R P
      30460ebd
    • K
      CI: Set binary swap source to master branch · daf37875
      Karen Huddleston 提交于
      This resource was compiling an old version of 5X, but that doesn't make
      sense for this branch. We will eventually pin this version at a 6 tag
      once we release and enable the binary swap tests.
      
      Also, update the gpaddon branch to pull from master instead of 5X
      Co-authored-by: NKaren Huddleston <khuddleston@pivotal.io>
      Co-authored-by: NJimmy Yih <jyih@pivotal.io>
      daf37875
    • H
      Fix scenario where streaming standby gets stuck at a continuation record. · f372675e
      Heikki Linnakangas 提交于
      If a continuation record is split so that its first half has already been
      removed from the master, and is only present in pg_wal, and there is a
      recycled WAL segment in the standby server that looks like it would
      contain the second half, recovery would get stuck. The code in
      XLogPageRead() incorrectly started streaming at the beginning of the
      WAL record, even if we had already read the first page.
      
      Backpatch to 9.4. In principle, older versions have the same problem, but
      without replication slots, there was no straightforward mechanism to
      prevent the master from recycling old WAL that was still needed by standby.
      Without such a mechanism, I think it's reasonable to assume that there's
      enough slack in how many old segments are kept around to not run into this,
      or you have a WAL archive.
      
      Reported by Jonathon Nelson. Analysis and patch by Kyotaro HORIGUCHI, with
      some extra comments by me.
      
      Discussion: https://www.postgresql.org/message-id/CACJqAM3xVz0JY1XFDKPP%2BJoJAjoGx%3DGNuOAshEDWCext7BFvCQ%40mail.gmail.com
      f372675e
    • A
      Improve performance of `merge_leaf_stats()` · 198a6bb8
      Abhijit Subramanya 提交于
      The previous algorithm to merge the HLL counters for leaf partitions had a time
      complexity of O(n^2) where n is the number of leaf partitions. In case where
      the number of partitions was high, resulting in poor performance of analyze on
      partitioned tables.  This patch fixes the performance issue by using a new
      algorithm to merge the leaf HLL counters in O(n) time.
      Co-authored-by: NOmer Arap <oarap@pivotal.io>
      Co-authored-by: NBhuvnesh Chaudhary <bchaudhary@pivotal.io>
      198a6bb8
  9. 19 12月, 2018 9 次提交