1. 12 5月, 2020 1 次提交
    • J
      Fix multiple definition linker error · ee7eb0e8
      Jesse Zhang 提交于
      Looks like we were missing an "extern" in two places. While I was at it,
      also tidy up guc_gp.c by moving the definition of Debug_resource_group
      into cdbvars.c, and add declaration of
      gp_encoding_check_locale_compatibility to cdbvars.h.
      
      This is uncovered by building with GCC 10 and Clang 11, where
      -fno-common is the new default [1][2] (vis a vis -fcommon). I could also
      reproduce this by turning on "-fno-common" in older releases of GCC and
      Clang.
      
      We were relying on a myth (or legacy compiler behavior, rather) that C
      tentative definitions act _just like_ declarations -- in plain English:
      missing an "extern" in a global variable declaration-wannabe wouldn't
      harm you, as long as you don't put an initial value after it.
      
      This resolves #10072.
      
      [1] "3.17 Options for Code Generation Conventions: -fcommon"
      https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Code-Gen-Options.html#index-tentative-definitions
      [2] "Porting to GCC 10" https://gcc.gnu.org/gcc-10/porting_to.html
      [3] "[Driver] Default to -fno-common for all targets" https://reviews.llvm.org/D75056
      ee7eb0e8
  2. 11 5月, 2020 2 次提交
  3. 09 5月, 2020 15 次提交
  4. 08 5月, 2020 3 次提交
    • P
      simplify the uao_crash_compaction_row test · 157abf3d
      Pengzhou Tang 提交于
      commit 5778f311 merged the AOVAC_DROP and AOVAC_CLEANUP phases
      to one phase called VACOPT_AO_POST_CLEANUP_PHASE, this makes the
      fault injector 'compaction_before_cleanup_phase' and 'compaction_
      before_segmentfile_drop' duplicated, we only need to keep one
      injector.
      157abf3d
    • P
      Fix flaky uao_crash_compaction_row test: part two · 9ed729db
      Pengzhou Tang 提交于
      In commit b00916e6, we added a new
      fault injector to wait segment 1 to finish the post cleanup and drop
      the dead ao segment files, however, the test is still flaky because
      segment 1 might delay the dropping of awaiting-drop segment files, this
      is caused by commit 5778f311 which changed the vacuum behavor:
      
      "if there are concurrent transactions running, even in READ COMMITTED
      mode, and even if they don't access the table at all, VACUUM will no
      longer be able to recycle compacted segfiles. Next VACUUM will clean
      them up, after the old concurrent transactions."
      
      in test uao_crash_compaction_row, VACUUM has concurrent transactions.
          1&:VACUUM crash_before_cleanup_phase;
          3:SELECT gp_wait_until_triggered_fault('compaction_before....
      it makes the status of awaiting-drop aoseg files in segment 1
      undetermined.
      
      To resolve this, we only check the awaiting-drop/compaction aoseg files
      and compaction aoseg files in segment 0 when the first VACUUM (with
      concurrent transactions) is performed, then we do VACUUM again (without
      concurrent transactions) to verify the awaiting-drop/compaction aoseg
      files in segment 1.
      
      the commit b00916e6 is useless after we changed the test, so revert
      it.
      Reviewed-by: NAsim R P <apraveen@pivotal.io>
      9ed729db
    • J
      Revert "Don't suppress unused-but-set variable warning." · f9aa578c
      Jesse Zhang 提交于
      On HEAD (commit 5cc2ea50) we're seeing a flurry of 78 warnings of
      unused-but-set-variable warnings using GCC 9. We were clearly not ready
      for this change. I was blindsided by my local choice of tools (Clang 10)
      when I thought we were warning-free.
      
      This reverts commit ec1f5f08.
      f9aa578c
  5. 07 5月, 2020 3 次提交
    • G
      Fix possible crash in COPY FORM on QEs · 5cc2ea50
      ggbq 提交于
      Target partitions need new ResultRelInfos and override previous
      estate->es_result_relation_info in NextCopyFromExecute(). The
      new ResultRelInfo may leave its resultSlot as NULL. If sreh is
      on, the parsing errors will be caught and loop back to parse
      another row; however, the estate->es_result_relation_info was
      already changed. This can cause crash.
      
      Reproduce:
      
      ```sql
      CREATE TABLE partdisttest(id INT, t TIMESTAMP, d VARCHAR(4))
      DISTRIBUTED BY (id)
      PARTITION BY RANGE (t)
      (
        PARTITION p2020 START ('2020-01-01'::TIMESTAMP) END ('2021-01-01'::TIMESTAMP),
        DEFAULT PARTITION extra
      );
      
      COPY partdisttest FROM STDIN LOG ERRORS SEGMENT REJECT LIMIT 2;
      1	'2020-04-15'	abcde
      1	'2020-04-15'	abc
      \.
      ```
      5cc2ea50
    • A
      Fix a unit test failure on ARM · 6f7cd0d0
      Amit Khandekar 提交于
      While generating the check_expected() function calls, the mocker.py
      script does not consider functions that have arguments with
      non-integral types like typedefs of structures. Due to this, a
      function appendPQExpBufferVA() which has va_list argument, causes
      compilation error "aggregate value used where an integer was expected"
      in src/test/unit/mock/backend/libpq/pqexpbuffer_mock.c
      since the check_expected() is called for the va_list argument as well.
      This is because in the check_expected() definition, the arg is cast to
      an integral type such as long, which is illegal if the arg type is a
      structure.
      
      Now, this error shows up on ARM, but not on x86.
      The probable explanation is : va_list is defined as a typedef of
      __builtin_va_list, and __builtin_va_list is internally defined
      by gcc compiler, rather than a typedef statement in one of the gcc
      header files. So we cannot know what __builtin_va_list is defined as,
      but since it shows up only on ARM, it must be defined to some
      structure on ARM, and to some integral type such as int or long, on
      x86.
      
      Fix is to handle va_list as a special case and skip check_expected()
      call for variadic functions that use va_list rather than "..."
      notation. An ideal and bigger fix should have been to handle function
      arguments of structure types, but it's better to do that as a
      separate item, since it needs more thought, and a solution is not
      known as of now.
      
      Author: Amit Khandekar
      6f7cd0d0
    • R
      Fix how must_gather is determined with LIMIT ALL. · 3c1fafbe
      Richard Guo 提交于
      If there is ORDER BY or DISTINCT in the query, we need to bring all the
      data to a single node by setting must_gather to be true. An exception is
      when there's a LIMIT or OFFSET clause, which would be handled later when
      inserting Limit node. Here to tell if there is any LIMIT or OFFSET
      clause, we should use limit_needed, instead of checking limitCount or
      limitOffset directly.
      
      Fixes issue #9746.
      Reviewed-by: NHeikki Linnakangas <hlinnakangas@pivotal.io>
      Reviewed-by: NEkta Khanna <ekhanna@pivotal.io>
      3c1fafbe
  6. 06 5月, 2020 15 次提交
    • J
      Support 'n_mod_since_analyze' in system view pg_stat_all_tables (#10008) · 411bded4
      Jinbao Chen 提交于
      The column n_mod_since_analyze was always 0 in the past. The root cause
      is that n_mod_since_analyze is generated on segments but we read it from
      master. Fix the sql and read the value from segments.
      411bded4
    • J
      Suppress missing format attribute warning for C++ · 942448b7
      Jesse Zhang 提交于
      As it turns out, Wmissing-format-attribute is a no-op in Clang. And GCC
      correctly errors out on a couple of printf-like functions in ORCA for
      not having the "format" annotation. To get past compilation, temporarily
      suppress the warning just for C++. When a forthcoming patch brings in
      the attribute we can revert this fix.
      942448b7
    • J
      Suppress Clang's register deprecation warning. · 9a472294
      Jesse Zhang 提交于
      The Greenplum ORCA translator is written in C++ and it includes a lot of
      Postgres headers. Clang started warning about the use of "register"
      keywords in C++ compilation units since release 3.4 (in 2014). There's
      not much we can do about the use of register in legitimate inline C
      functions, and the translator code has produced those noisy warnings
      since. It's time we suppressed it.
      
      Examples of warning:
      In file included from CTranslatorDXLToPlStmt.cpp:27:
      In file included from ../../../../src/include/cdb/partitionselection.h:20:
      In file included from ../../../../src/include/nodes/execnodes.h:20:
      In file included from ../../../../src/include/access/heapam.h:22:
      In file included from ../../../../src/include/storage/lock.h:23:
      In file included from ../../../../src/include/storage/lwlock.h:22:
      ../../../../src/include/storage/s_lock.h:226:2: warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]
              register slock_t _res = 1;
              ^~~~~~~~~
      In file included from CTranslatorDXLToPlStmt.cpp:27:
      In file included from ../../../../src/include/cdb/partitionselection.h:20:
      In file included from ../../../../src/include/nodes/execnodes.h:20:
      In file included from ../../../../src/include/access/heapam.h:22:
      In file included from ../../../../src/include/storage/lock.h:23:
      In file included from ../../../../src/include/storage/lwlock.h:23:
      In file included from ../../../../src/include/port/atomics.h:69:
      ../../../../src/include/port/atomics/arch-x86.h:143:2: warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]
              register char _res = 1;
              ^~~~~~~~~
      2 warnings generated.
      9a472294
    • J
      Preserve user's CXXFLAGS. · e30f913e
      Jesse Zhang 提交于
      This makes it possible for a user to specify C++ compiler flags (for
      example CXXFLAGS='-O0') as an argument to configure, or as an
      environment variable.
      
      [Resolves #9980]
      e30f913e
    • J
      Stop deriving C++ flags from CFLAGS. · 381e1675
      Jesse Zhang 提交于
      Commit 2e387d0e "Disable aggressive loop optimizations in GCC 4.8+"
      created a precedent of using autoconf to manipulate CXXFLAGS. We've then
      been getting more flags by stealing them wholesale (modulo checking for
      compiler support) from CFLAGS since commit 9e2dab0a "Derive CXXFLAGS
      from CFLAGS.". This was neat in that it was low-maintenance, and it has
      practically served us well for the past 3 years. However, our current
      simplistic CXXFLAGS manipulation leaves a lot to be desired:
      
      1. It's implicit, giving us very little control over what flags to *not*
      include in CXXFLAGS. To wit, -Wmissing-prototypes is inapplicable to C++
      but still included in CXXFLAGS.
      
      2. Negative flags are not well supported. Some compilers -- notably GCC
      -- do not reject a negative warning flag (e.g. -Wno-fff). Compiler
      detection for such flags needs to be done in the positive form before
      adding the negative flag.
      
      3. It neglects CXXFLAGS the user specifies (either in the environment or
      as an argument to configure)
      
      This patch replaces the implicit derivation with explicit setting of
      CXXFLAGS. I've tested that this maintains behavior parity (modulo losing
      -Wmissing-prototypes, which is intentional). I've preserved the
      Greenplum idiosyncrasy of defaulting O3.
      
      [resolves #9979]
      
      Note that sooner or later we're gonna merge with upstream Postgres 11
      where C++ support is added to configure. The macro name
      PGAC_PROG_CXX_CXXFLAGS_OPT is deliberately (or it might have been a
      happenstance) chosen to be *different* from the upstream macro name so
      as to make "git blame" more useful and make it easier to wholesale take
      the upstream changes when the merge happens.
      381e1675
    • J
      Modernize PGAC_PROG_CXX_CXXFLAGS_OPT. · e42dae7d
      Jesse Zhang 提交于
      We introduced PGAC_PROG_CXX_CXXFLAGS_OPT as a straightforward adaptation
      of PGAC_PROG_CC_CFLAGS_OPT to C++ in commit 2e387d0e ("Disable
      aggressive loop optimizations in GCC 4.8+ (#1200)"). They have now
      drifted apart quite a bit.
      
      This patch introduces PGAC_PROG_CXX_VAR_OPT in the image of
      PGAC_PROG_CC_VAR_OPT and rewrote PGAC_PROG_CXX_CXXFLAGS_OPT as a simple
      wrapper on top. One immediate benefit of this change is that the flag
      detection is now cached, so it'll be quite a bit faster on re-configure.
      We haven't removed the site of use that adds C++ flags in a loop from
      the main configure.in yet; the "eval"s are necessary to accommodate the
      non-literal arguments passed from configure.in.
      
      I've preserved the wording "if $CXX supports ..." instead of following
      the upstream change from "if" to "whether".
      
      NOTE: I've preserved the macro name PGAC_PROG_CXX_CXXFLAGS_OPT (which is
      different from PGAC_PROG_CXX_CFLAGS_OPT from Postgres 11+) both to
      minimize the diff, and to make it abundantly clear when we merge with
      Postgres 11 and wholesale take it from upstream.
      e42dae7d
    • J
      Aligns CXXFLAGS initialization with CFLAGS. · d53b91b4
      Jesse Zhang 提交于
      This is effectively the same as what we're currently doing, but more
      aligned with how upstream manipulates CFLAGS: we take care of the debug
      symbol and optimization level flags, so we want to discard the
      C[XX]FLAGS we get from AC_PROG_{CC,CXX}.
      
      This still doesn't quite feel right (because of the "borrowing" between
      CFLAGS and CXXFLAGS), but it's a good behavior-parity commit point.
      d53b91b4
    • J
      Output CXXFLAGS together with CFLAGS in configure. · 95214d4e
      Jesse Zhang 提交于
      This makes it much more clear to the user (read: me) what configure has
      settled on for C++ compilation flags. This prepares us for a forthcoming
      changes which changes how we manipulate CXXFLAGS in configure. This is
      written to *always* output CXXFLAGS even if we dis-configure C++ parts
      (gpcloud, ORCA, etc) to match the unconditional AC_PROG_CXX. And even
      without the immediately following change, this change has merits on its
      own.
      95214d4e
    • J
      Remove duplicate flags from configure output. · 8e68835c
      Jesse Zhang 提交于
      Upstream commit 2c5d6f1f (Lane "Include $cc_string in the info
      reported by a configure run.") has moved the compiler flags output
      section down but we most likely mismerged it during 9.2 merge (commit
      598f4b0e "Merge with PostgreSQL 9.2beta2.")
      
      To keep behavior parity we're keeping the LIBS output line here. This
      seems actually still useful, and if we later find it less valuable it
      should be trivial to take it out.
      
      Before:
      
      checking whether ccache clang-10 supports -Wl,--as-needed... (cached) yes
      checking whether ccache clang-10 supports -Werror=uninitialized... (cached) yes
      checking whether ccache clang-10 supports -Werror=implicit-function-declaration... (cached) yes
      configure: using CFLAGS=-Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -ggdb -O3 -std=gnu99  -Werror=uninitialized -Werror=implicit-function-declaration
      configure: using CPPFLAGS= -D_GNU_SOURCE
      configure: using LDFLAGS=-fuse-ld=lld  -Wl,--as-needed
      configure: using LIBS=-lxerces-c -lbz2 -lrt -lssl -lcrypto -lzstd -lz -lreadline -lrt -lcrypt -ldl -lm
      configure: using compiler=clang version 10.0.0-++20200412073924+50d7e5d5e7d-1~exp1~20200412054519.133
      configure: using CFLAGS=-Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -ggdb -O3 -std=gnu99  -Werror=uninitialized -Werror=implicit-function-declaration
      configure: using CPPFLAGS= -D_GNU_SOURCE
      configure: using LDFLAGS=-fuse-ld=lld  -Wl,--as-needed
      configure: creating ./config.status
      
      After:
      
      checking whether ccache clang-10 supports -Wl,--as-needed... (cached) yes
      checking whether ccache clang-10 supports -Werror=uninitialized... (cached) yes
      checking whether ccache clang-10 supports -Werror=implicit-function-declaration... (cached) yes
      configure: using compiler=clang version 10.0.0-++20200412073924+50d7e5d5e7d-1~exp1~20200412054519.133
      configure: using CFLAGS=-Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -ggdb -O3 -std=gnu99  -Werror=uninitialized -Werror=implicit-function-declaration
      configure: using CPPFLAGS= -D_GNU_SOURCE  -I/home/pivotal/.opt/xerces/include
      configure: using LDFLAGS=-fuse-ld=lld  -L/home/pivotal/.opt/xerces/lib -Wl,--as-needed
      configure: creating ./config.status
      8e68835c
    • J
      Don't suppress unused-but-set variable warning. · ec1f5f08
      Jesse Zhang 提交于
      With the last bit of unused variable warning knocked out (I think), we
      can now build with the default warnings enabled so that whenever a new
      unused variable creeps in, it'll stand out (as envisioned in commit
      0675ad72 "Remove -Werror, replace with more specific -Wno-*
      options.").
      ec1f5f08
    • J
      Annotate assert-only variable · e4a616e0
      Jesse Zhang 提交于
      This should knock out the last instance of "unused variable" compiler
      warning.
      e4a616e0
    • J
      Drop trailing slashes in "subdir". · f90dd34f
      Jesse Zhang 提交于
      While functionally harmless, the trailing slashes seem to contradict our
      coding conventions. In addition, it leads to double slashes when we
      compute the linking command.
      
      While we're at it, also correct the trailing slashes in subdir for 3
      pre-existing Makefiles (fsync, heap_checksum, and walrep regress tests).
      f90dd34f
    • J
      Use CXXFLAGS instead of CPPFLAGS for -std and -W* . · d51ecabe
      Jesse Zhang 提交于
      Semantically, those are CXXFLAGS, not CPP flags (which typically deal
      with -D and -I stuff). Practically, this becomes a problem when we try
      to turn off certain warnings because CPPFLAGS come after CXXFLAGS and
      the order sensitivity of -Werror and -Wno-* flags won't mix well.
      d51ecabe
    • J
      Remove repetition in gpos Makefile · 20719067
      Jesse Zhang 提交于
      In a forthcoming commit we're gonna tweak some of these flags. To
      prevent duplication, consolidate them through Makefile inclusion first.
      This also addresses a FIXME left from commit 3f3f4a57 "Fix configure
      and cmake to build ORCA with debug".
      20719067
    • J
      Enable ccache on Travis CI (#9951) · e79b81e5
      Jesse Zhang 提交于
      While working on https://github.com/greenplum-db/gpdb/pull/9937 ("Build
      ORCA with C++14."), I realized we took out ccache from Travis CI. This
      patch set brings it back with some additional TLC.
      
      Highlights:
      
      * Remove deprecated [1][2] key "sudo: false"
      
      * Set Travis language to C++
      
        Travis propagates the environment variables CXX (in addition to CC,
        which is exported when the language is C) when the language is C++.
        This prepares us for a forthcoming change to enable ORCA build in
        Travis.
      
      * Care is taken to ensure ccache is actually used on macOS.
      
        The default settings in Travis somehow neglected it
        (travis-ci/travis-build#655) so we compensate for it.
      
      * Similar care is taken to ensure we use ccache with Clang.
      
        The version of ccache used in Travis is just slightly older than when
        automatic Clang symlink supported was added.
      
      * Show ccache stats (hit rate and more) for each build
      
      Reference:
      [1] https://blog.travis-ci.com/2018-11-19-required-linux-infrastructure-migration
      [2] Job config linting warning from Travis
      e79b81e5
  7. 05 5月, 2020 1 次提交
    • L
      docs - new pxf IGNORE_MISSING_PATH option (#10010) · 86bab74b
      Lisa Owen 提交于
      * docs - new pxf IGNORE_MISSING_PATH option
      
      * reword default case
      
      * add IGNORE_MISSING_PATH info to relevant profiles
      
      * the action to take
      
      * try to describe why pxf behaviour is not optimal
      86bab74b