Conversation
) When resource group is enabled, StartTransaction() may take a catalog snapshot while assigning a resource group, leaving a valid xmin in MyProc. That fails the assertion in set_indexsafe_procflags() that the process must not advertise an xmin when running "CREATE INDEX CONCURRENTLY". Fix it at the point the invariant lives: set_indexsafe_procflags() now invalidates any stale catalog snapshot before asserting, so every REINDEX CONCURRENTLY phase clears the xmin ahead of setting PROC_IN_SAFE_IC.
diskquota is the only component in the tree that requires a CMake
newer than 3.16, so building with --with-diskquota fails on distros
whose default CMake is older. On Ubuntu 20.04 (CMake 3.16.3) the
build stops at:
CMake Error at CMakeLists.txt:1 (cmake_minimum_required):
CMake 3.20 or higher is required. You are running version 3.16.3
make[1]: *** [Makefile:52: all-diskquota-recurse] Error 2
Every other first-party CMakeLists in the tree already declares 3.12
or lower, so lowering diskquota's two declarations to 3.16 brings it
in line with the rest and does not raise the bar anywhere else.
Neither declaration needed to be as high as it was:
* CMakeLists.txt claimed 3.20 for cmake_path, but cmake_path is never
called anywhere in the diskquota tree. The comment was stale.
* cmake/Regress.cmake claimed 3.17 for CMAKE_CURRENT_FUNCTION_LIST_DIR,
which is used to locate regress_loop.sh and regress_show_diff.sh.
That variable is replaced with REGRESS_CMAKE_LIST_DIR, captured from
CMAKE_CURRENT_LIST_DIR when the module is included, which resolves
to the same directory and is available in every supported CMake.
Lowering the version alone would not have been enough: on 3.16
CMAKE_CURRENT_FUNCTION_LIST_DIR expands to an empty string without
error, so the extension would still compile while installcheck
silently pointed at /regress_loop.sh.
One optional code path, guarded by DISKQUOTA_LAST_RELEASE_PATH, calls
file(ARCHIVE_EXTRACT) and so still needs CMake 3.18. The in-tree build
never sets that variable; a comment now records the requirement.
Assisted-by: Claude Code
* PAX: include bloomfilter columns in DELETE stats projection DeleteWithVisibilityMap projected only minmax_columns into the reader, then asked UpdateStatsInAuxTable to refresh bloomfilter stats too. When a bloomfilter column was not also a minmax column, the reader did not materialize it, and MicroPartitionStats::AddRow dereferenced an uninitialized slot value, crashing the segment with SIGSEGV. Project the union of minmax and bloomfilter column indexes, while keeping the original lists passed to UpdateStatsInAuxTable so per-stat semantics are unchanged. Hoist the GetBloomFilterColumnIndexes() call out of the per-block loop. Fixes apache#1749
…amp join selectivities
On an assert-enabled build the following query crashed the QD with
FailedAssertion("pselec >= 0.0 && pselec <= 1.0", costsize.c:5506):
CREATE TABLE m1(c0 inet);
CREATE TABLE m2(c0 inet);
INSERT INTO m2 VALUES ('88.147.138.141'), ('76.163.212.11'), ('214.10.65.144');
ANALYZE m1, m2;
SELECT COUNT(*) FROM ONLY m1 LEFT OUTER JOIN m2 ON true
WHERE (m1.c0 IS NOT NULL)
OR (m2.c0 BETWEEN SYMMETRIC '75.175.243.19' AND '230.9.216.68');
The pushed-down WHERE clause is an OR of an IS NOT NULL test on the
outer side (selectivity 0.999, no stats on the empty table) and a range
pair on the nullable side. When clauselist_selectivity() merges the
range pair (hibound + lobound - 1) it adds back the column's null
fraction to undo the double exclusion of NULLs, and it does so by
calling nulltestsel(IS_NULL, ..., jointype). GPDB's nulltestsel()
special-cases IS NULL under an outer join and returns 0.5 (a guess for
the anti-join "WHERE inner.col IS NULL" pattern, removed upstream in
e006a24), so the range pair comes out as 0.99 + 0.5 = 1.49 instead of
0.99 + 0.0. Combining that with the IS NOT NULL arm via
s1 + s2 - s1*s2 gives 1.00049, which adjust_selectivity_for_nulltest()
asserts on.
Two changes:
1. In clauselist_selectivity_ext(), ask nulltestsel() for the column's
real null fraction by passing JOIN_INNER. The range-pair correction
is a statistical adjustment, not an IS NULL predicate evaluated at
the join level, so the outer-join guess never belonged here. Before
this, every range pair evaluated under an outer join was inflated by
an absolute 0.5; after it, a LEFT JOIN ON clause with a range
condition gets the same estimate as the equivalent inner join. This
matches upstream behaviour, where nulltestsel() ignores jointype.
2. Selectivities are probabilities, so also clamp jselec and pselec to
[0, 1] in calc_joinrel_size_estimate() before handing them to
adjust_selectivity_for_nulltest(), so round-off in the OR combination
can never trip the assertion again.
Add the SQLancer query to bfv_planner as a regression test.
Fixes apache#1950
The clients target copies the shell script that sets GPHOME_CLIENTS into the package it assembles: cp -f client/scripts/greenplum_$@_path$(SCRIPT) $(CLIENTSINSTLOC)/ With $@ expanding to "clients" and SCRIPT to ".sh" that resolves to client/scripts/greenplum_clients_path.sh, which does not exist. The file was renamed to cloudberry_clients_path.sh during the rebranding and this reference was not updated with it, so the target would fail here if anyone reached it. Nobody has: the top-level GNUmakefile deliberately steps over gpAux/Makefile, and the body of the clients target is skipped unless BLD_TARGETS contains "clients", which nothing in the tree sets. So this is a latent break rather than a live one, and fixing it costs a word. Verified with `make -C gpAux -n clients BLD_TARGETS=clients` on Rocky 9. Before, the expanded command reads cp -f client/scripts/greenplum_clients_path.sh ... and after cp -f client/scripts/cloudberry_clients_path.sh ... which matches the file in the tree and copies successfully when run from gpAux/, the directory make uses. Note that gpMgmt/bin/gpload, gpload.py and gpdirtableload still look for greenplum_loaders_path.sh under $GPHOME_LOADERS. Those are deliberate compatibility lookups against a separately installed Greenplum loaders package, not references to anything in this repository, so they are left alone. Assisted-by: Claude Code Backpatch-through: REL_2_STABLE
CursorICHistoryTable::add() and prune() compute their bucket index as a uint32, but purge() iterates the table with a uint8 index. The table size comes from gp_interconnect_cursor_ic_table_size, which defaults to 128 but may be raised up to 102400, and the GUC's own description tells users to raise it when UDFs with many concurrent cursors hang. Any value above 255 turns the purge loop into an infinite loop (uint8 wraps before reaching size) while holding the interconnect lock, which also wedges the receiver thread. Use uint32 to match the rest of the table.
Four third-party components that are compiled into and shipped with
Cloudberry were never registered in the root LICENSE file:
- gpcontrib/pg_hint_plan/* BSD 3-Clause (NTT)
- gpMgmt/bin/lib/pexpect/__init__.py ISC (Noah Spurrier)
gpMgmt/bin/lib/pexpect/pxssh.py
- gpcontrib/gpcloud/lib/http_parser.* MIT (Joyent / NGINX)
- gpcontrib/gpcloud/lib/ini.* MIT (rxi)
All four arrived with the Greenplum code base, so they are registered
under the existing "The Greenplum Database software includes" section.
Their upstream license texts are added under licenses/, copied from the
files themselves (pg_hint_plan from its own COPYRIGHT file).
The pexpect entry deliberately names the two upstream files rather than
using a gpMgmt/bin/lib/pexpect/* wildcard: the third file in that
directory, Makefile, is our own build glue and is not under the pexpect
license. gpcontrib/pg_hint_plan/ is a pristine upstream subtree with no
project-authored files, so a wildcard is accurate there.
Note that pom.xml already carries a dedicated Apache RAT matcher for
rxi's ini ("MIT (RXI-derived)"), so the component was known to the audit
tooling but had never been reflected in LICENSE.
Auditing every path referenced by LICENSE also turned up a stray trailing
colon after src/pl/plperl/ppport.h, corrected here. The same audit found
that LICENSE pointed at dependencies/yyjson while the directory is
dependency/yyjson; apache#1995 has since corrected that independently, so this
branch no longer carries it.
Verified with `mvn clean verify -Drat.consoleOutput=true` on Rocky 9:
Unapproved: 0, unknown: 0, generated: 1, approved: 5657 licenses.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
src/port/glob.c, src/port/glob.h and src/include/port/win32_msvc/glob.h
carry the original 4-clause BSD license from the Regents of the
University of California, with the advertising clause intact. The ASF
lists that variant under Category X, which may not be distributed in
source or binary form, and the files have shipped in every release so
far. Reported by Justin Mclean during the 2.2.0-rc1 IPMC vote.
The files are not PostgreSQL code and can simply go away:
- PostgreSQL has no glob.c, glob.h or win32_msvc/glob.h on master,
REL_16_STABLE or REL_14_STABLE; upstream's win32_msvc directory
holds only dirent.h, sys, unistd.h and utime.h. src/port/glob.c
still carries a PHP CVS keyword ($Id: glob.c,v 1.7 2007/11/10
09:56:37 dmitry Exp $), so it came from PHP's win32 glob by way of
Greenplum, and src/port/glob.h arrived with "Move gpfdist to gpdb
core".
- The only consumers are the Windows gpfdist builds. In
src/bin/gpfdist/Makefile the object was added under
ifeq ($(PORTNAME),win32), and src/bin/gpfdist/CMakeLists.txt is a
standalone MSVC project (/MP /wd4996 compiler switches, ws2_32 and
Crypt32, libapr-1.lib) that nothing in the build system invokes.
Cloudberry neither supports nor produces Windows builds.
- The two places that really use glob, src/backend/utils/adt/dbsize.c
and src/backend/utils/misc/fstream/fstream.c, include <glob.h> and
resolve to the system header.
The win32 block in the gpfdist Makefile is removed whole rather than
trimmed: its -I$(top_builddir)/src/port was added by the same commit
that added src/port/glob.h and existed only to find that header.
The LICENSE section covering these files and licenses/LICENSE-glob.txt
go with them. Note that the section listed only two of the three files;
src/port/glob.h was never registered.
Checked on Rocky 9 that no reference to glob.c, glob.o or LICENSE-glob
remains anywhere in the tree, and that gpfdist builds byte-identically
before and after (133152 bytes) and still runs; PORTNAME is linux there,
so the removed block never fired to begin with.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
Justin Mclean flagged three Thumbs.db files in the source archive during the 2.2.0-rc1 IPMC vote. One of them sits under gpAux/client, and looking at what else is in there turned up something worse. gpAux/client/install is the old Greenplum Windows client MSI installer: a WiX project, a handful of .ico and .bmp resources, two .bat scripts and license.rtf. That last file is 853 KB of VMware End User License Agreement, last updated 03 May 2021, a proprietary commercial agreement shipped inside an Apache source release. greenplum-clients.wxs also still declares Manufacturer="Pivotal, Inc." and Manufacturer="Greenplum, Inc.". Nothing builds it. gpAux/Makefile has a clients target that would consume these files, but the top-level GNUmakefile deliberately steps over gpAux/Makefile -- it is the entry point of the old enterprise build -- the target's body is skipped unless BLD_TARGETS contains "clients", which nothing in the tree sets, and it would fail anyway: it copies client/scripts/greenplum_clients_path.sh, a name that no longer exists since the file was renamed to cloudberry_clients_path.sh without the Makefile being updated. Cloudberry does not support or produce Windows client packages and there are no plans to, so gpAux/client/install goes in full, along with gpAux/client/scripts/cloudberry_clients_path.bat. gpAux/client/scripts/cloudberry_clients_path.sh stays. It is 47 lines of Linux shell that set GPHOME_CLIENTS and the library paths, it carries no licensing problem, and it is the only description we have of what a client-only package would need if one is ever built again. Also removed: - gpcontrib/orafce/doc/orafce_documentation/gif/Thumbs.db - gpcontrib/orafce/doc/sql_migration/gif/Thumbs.db These are Windows Explorer thumbnail caches that came along with orafce's documentation images, 248 KB and 27 KB of binary data with no purpose in a source tree. pom.xml loses the six Apache RAT exclusions that now point at nothing, and the **/Thumbs.db exclusion, which is what kept RAT quiet about these files. Dropping it means a future Thumbs.db gets flagged instead of silently excluded. The exclusion for cloudberry_clients_path.sh stays with the file. .gitignore now lists Thumbs.db and ehthumbs.db so they do not come back. Verified on Rocky 9 against a fresh checkout of main: `mvn clean verify -Drat.consoleOutput=true` gives Unapproved: 0, unknown: 0, generated: 1, approved: 5651 licenses, BUILD SUCCESS. Assisted-by: Claude Code Backpatch-through: REL_2_STABLE
src/backend/libpq/sha2.h carries Aaron D. Gifford's BSD 3-Clause notice and
is registered in LICENSE under it, but nothing in the tree can reach the
file:
- no translation unit includes a bare "sha2.h", in src/backend/libpq or
anywhere else
- #include "libpq/sha2.h" cannot resolve to it either, because headers
are found under src/include and there is no src/include/libpq/sha2.h
The same upstream code does live in the tree, at src/common/sha2.c and
src/common/sha2_int.h, both carrying the same Aaron D. Gifford copyright
and the same OpenBSD and adg version keywords. Those two are real:
sha2_int.h is included by sha2.c, cryptohash.c, cryptohash_openssl.c and
sm3.c, and sha2.c is in src/common's object list.
So the header is a leftover duplicate. Delete it, and repoint the LICENSE
entry at the copies that are actually there rather than dropping the entry
along with the file: the BSD 3-Clause code itself has not gone anywhere,
only this stale copy of it has, and licenses/LICENSE-bsd3-sha2.txt still
matches the notice on both remaining files.
Note that the five "common/sha2.h" includes elsewhere in the tree refer to
src/include/common/sha2.h, a different and PostgreSQL-licensed file, not
to either of these.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
Cloudberry derives a large part of its code from Greenplum Database, which
LICENSE has said since the beginning:
This product contains significant parts that were originally based on
software from VMware, Inc. or its affiliates.
Greenplum Database
Copyright (c) 2004-2020 VMware, Inc. or its affiliates. All Rights
Reserved.
NOTICE has never mentioned it. Greenplum is Apache-2.0 licensed and ships
a NOTICE file whose first lines are exactly that attribution, so section
4(d) of the license asks us to carry it forward.
The three satellite repositories already do. cloudberry-backup,
cloudberry-go-libs and cloudberry-pxf each name VMware in their NOTICE,
which leaves the repository with the deepest Greenplum lineage as the only
one that does not. This commit uses their wording and layout so the four
read the same way.
Only the attribution is taken. greenplum-db/gpdb-archive's NOTICE runs to
363 lines because it also carries the full license texts of PostgreSQL,
Python, pexpect, libsocket and curl; that material is not an attribution
notice, infra.apache.org/licensing-howto.html asks that NOTICE not collect
it, and Cloudberry already covers those components in LICENSE and
licenses/ where they belong.
NOTICE also gains the trailing newline it was missing.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
PyYAML 5.4.1 no longer builds its C extension on Rocky 10. Its
pre-generated _yaml.c declares the libyaml read/write handlers with
char * where libyaml wants unsigned char *, and GCC 14 turned that from
a warning into an error:
yaml/_yaml.c:3598:52: error: passing argument 2 of 'yaml_parser_set_input' from incompatible pointer type [-Wincompatible-pointer-types]
/usr/include/yaml.h:1374:30: note: expected 'int (*)(void *, unsigned char *, size_t, size_t *)' but argument is of type 'int (*)(void *, char *, size_t, size_t *)'
Error compiling module, falling back to pure Python
The build then completes with the slow pure-Python parser, so gpload
still works; this is quality rot rather than a hard failure. Note that
the CFLAGS="-w" already passed here cannot suppress it, since -w only
silences warnings and GCC 14 makes this an error.
While in here, the other two bundled packages are equally old:
PyGreSQL 5.2 is from 2020-06, psutil 5.7.0 from 2020-02.
PyGreSQL 5.2 -> 5.2.5
psutil 5.7.0 -> 7.2.2
PyYAML 5.4.1 -> 6.0.1
The ceilings are set by Python 3.6, which Rocky 8 ships as its system
python3 and which is still a supported build platform:
* PyYAML 6.0.1 is the newest release declaring >=3.6; 6.0.2 moved to
>=3.8.
* PyGreSQL stays on the 5.x line on purpose. 6.x requires >=3.7 and its
setup.py refuses older interpreters outright ("Sorry, PyGreSQL 6.1.0
does not support this Python version"). 5.2.5 is also a drop-in, which
matters because gpMgmt imports the top-level pg and pgdb modules in
eleven files.
* psutil is the exception: even 7.2.2 still declares >=3.6, and the only
API gpMgmt uses is virtual_memory(), Process(), Process.children(),
Process.memory_info(), wait_procs(), pid_exists() and NoSuchProcess,
none of which changed in 6.x or 7.x.
Verified on all five build platforms (Rocky 8/9/10, Ubuntu 22.04/24.04)
by running the real download-python-deps, pyyaml, psutil and pygresql
targets: all succeed with no errors and no pure-Python fallback, and
importing yaml, psutil, pg and pgdb out of the resulting gpMgmt/bin/ext
exercises the calls listed above. Rocky 8 covers the Python 3.6 end and
Rocky 10 the GCC 14 end.
This is not a security fix: OSV reports no known advisories against
either the old or the new versions.
Two things worth knowing before the next bump. From PyYAML 6.0.2 and
PyGreSQL 6.0.1 onwards the sdist filename is lowercase, which the
hardcoded PyYAML-$(PYYAML_VERSION).tar.gz style URLs and directory names
here do not handle; since curl runs without -f the 404 body lands in the
tarball and the failure surfaces at tar time. And the "cython<3.0.0"
pin in download-python-deps is load-bearing: PyYAML re-cythonizes when
Cython is importable, and both 5.4.1 and 6.0.1 die with
"AttributeError: cython_sources" under Cython 3.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
yyjson sits in a top-level dependency/ directory as a git submodule, and
it is the only thing in there. It does not need to be in the source
tree at all:
- Nothing outside contrib/pax_storage/src/cpp/manifest/manifest.c uses
it. Cloudberry core does not depend on yyjson, which answers the
question raised on this PR about why it sat at the top level.
- It is compiled only when USE_MANIFEST_API is ON and USE_PAX_CATALOG
is OFF. Both default the other way and nothing in the tree turns
them on, so no build we ship compiles yyjson: pax.so and
libpaxformat.so contain no yyjson symbols and manifest.c produces no
object file.
Carrying it anyway is not free. dependency/yyjson is 9.3 MB across 804
files, of which 3.0 MB and 201 files are generated doxygen output, and
pom.xml needs 249 RAT exclusions to cope with it, 200 of them for the
docs alone. That doxygen output also bundles jQuery, jQuery UI, jQuery
UI Touch Punch, doxygen-awesome-css and Doxygen's own helper scripts,
none of which the root LICENSE accounts for. Justin Mclean raised the
LICENSE gap during the 2.2.0-rc1 IPMC vote; fetching yyjson on demand
removes the components rather than documenting them.
CMake now fetches the dependency during configuration, as suggested by
Ed Espino on this PR. This follows what gpMgmt/bin/Makefile already
does for psutil, PyYAML and PyGreSQL, which are downloaded at build time
for the same reason. The version is a release tag rather than the
previous submodule pointer, which was 61c03f6 "Try fix github action",
not a release at all.
Ed's draft also tried a system yyjson through find_package first. That
path is left out: it sets neither yyjson_SOURCE_DIR, which the include
path needs, nor a target named yyjson, which pax.cmake links, because an
installed yyjson exports yyjson::yyjson. Since the previous behaviour
was always to build yyjson from source, always fetching keeps the change
behaviour-preserving, and a branch no image can exercise would only rot.
The LICENSE entry, licenses/LICENSE-yyjson.txt and all 249 RAT
exclusions go away with the directory. This is rebased on top of the
boost licence fix, apache#1995 on main and apache#1987 on REL_2_STABLE, which
corrected the entry from dependencies/yyjson to dependency/yyjson; the
whole entry is removed either way, and every hunk now applies cleanly to
REL_2_STABLE as well.
Verified on Rocky 9:
- `mvn clean verify -Drat.consoleOutput=true` against a fresh checkout
of main with this applied gives Unapproved: 0, unknown: 0,
generated: 1, approved: 5650 licenses, BUILD SUCCESS, so dropping the
249 exclusions leaves nothing unaccounted for.
- The default PAX build succeeds with no network access at all, which
is the configuration every release build uses.
- With USE_MANIFEST_API=ON USE_PAX_CATALOG=OFF, CMake fetches yyjson
0.12.0, builds libyyjson.so.0.12.0, puts -I.../_deps/yyjson-src/src
on the compile line that manifest.c uses and libyyjson.so.0.12.0 on
the link line.
- That configuration then fails in
micro_partition_iterator_manifest.cc, which still uses the
pre-PostgreSQL-16 rd_node field. An unpatched tree with the
submodule fails at the same two lines with the same error, so the
breakage predates this change and confirms how long the manifest API
path has gone unbuilt.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
Justin Mclean noted during the 2.2.0-rc1 IPMC vote that the release has no crypto export notice despite shipping contrib/pgcrypto, and that Cloudberry does not appear on https://www.apache.org/licenses/exports/. infra.apache.org/crypto.html requires the notice in the README of each distribution, so add it there, listing the components of the release that involve cryptography: OpenSSL for TLS and SCRAM-SHA-256, optional GSSAPI and LDAP over TLS, and the bundled pgcrypto extension. The build links against the operating system's OpenSSL and does not ship it, which the notice states because it determines who is named as manufacturer in the BIS notification. This commit only covers the README. Two steps remain, and neither lives in the source tree: - adding Cloudberry to https://www.apache.org/licenses/exports/, which is a pull request against apache/www-site, file data/eccn/eccnmatrix.yaml. Podlings do not get their own top-level entry there; sixteen of them sit as products under a single - href: https://incubator.apache.org/ name: Apache Incubator Project contact: Apache Incubator PMC <private@incubator.apache.org> umbrella, so Cloudberry belongs under that one, with the contact inherited from the Incubator PMC rather than one of our own lists. - sending the ECCN 5D002 notification to crypt@bis.doc.gov, enc@nsa.gov and web_site@bis.doc.gov. This is a named legal notification submitted on behalf of the ASF. The procedure describes it as coming from the PMC chair, and for a project in incubation the PMC is the Incubator PMC; infra.apache.org/crypto.html says nothing about podlings, so who sends it is a question for our mentors. The site build generates the text of this email from the matrix entry, so it does not have to be written by hand. Per the same page those are supposed to happen before the code is posted publicly, so they are overdue rather than new; the notification names the product without a version and does not need repeating per release. Assisted-by: Claude Code Backpatch-through: REL_2_STABLE
Python 3.12 promoted "invalid escape sequence" from a DeprecationWarning
(hidden by default) to a SyntaxWarning that is printed whenever the
module is compiled. On distributions shipping Python 3.12 or newer --
Rocky Linux 10, Fedora 40+, Ubuntu 24.10+ -- the management utilities
therefore print warnings before doing any work:
/usr/local/cloudberry-db/lib/python/gppylib/util/ssh_utils.py:268:
SyntaxWarning: invalid escape sequence '\ '
/usr/local/cloudberry-db/bin/lib/pexpect/pxssh.py:105:
SyntaxWarning: invalid escape sequence '\['
/usr/local/cloudberry-db/bin/lib/pexpect/pxssh.py:109:
SyntaxWarning: invalid escape sequence '\$'
/usr/local/cloudberry-db/bin/lib/pexpect/pxssh.py:110:
SyntaxWarning: invalid escape sequence '\$'
The four above come from gpsync/gpssh/gpssh-exkeys and are the ones
users see; compiling everything under gpMgmt/ turns up 41 such literals
in 16 files. Nothing misbehaves today -- CPython leaves an unrecognised
escape in the string as-is, which happens to be what a regex or a shell
snippet wants -- but the warnings are noise on stderr, and the Python
docs say these sequences will become a SyntaxError in a future release.
Fix the literals rather than silencing the warning:
* a literal whose every backslash escape is invalid gets an r prefix
* a literal that mixes valid and invalid escapes has the invalid
backslash doubled instead, since r would also change the meaning
of the valid ones
Both transformations leave the literal's runtime value byte-for-byte
identical, which was verified mechanically: for each touched file the
ordered list of str/bytes constants in the AST is unchanged against the
parent commit, and compiling the file under -W error::SyntaxWarning is
now clean.
Two of the rewrites are worth a reviewer's eye:
* gpstate_utils.py:79 and replication_slots_utils.py:31 are the
mixed-escape case, so they read '\\%' and '\\A' now.
* the '\A' in replication_slots_utils.py looks like a stray keystroke
in a run of shell line continuations -- the shell it is handed sees
"&& A ./demo_cluster.sh" and tries to run A. That is pre-existing
behaviour, so this commit preserves it exactly rather than quietly
changing what the behave step does; it wants a separate fix.
gpMgmt/bin/lib/pexpect is a vendored copy of pexpect 3.3; upstream
pexpect made these same literals raw long ago.
27 more files under src/ and contrib/ (mostly gporca and try_convert
developer scripts) have the same problem and are left for a follow-up.
The Incubator releasecheck tool reports binary files in the 2.2.0-rc1
source archive. Looking at each one, only two are avoidable.
gpMgmt/demo/gppkg/data/ held a .deb and an .rpm, and they are output of
the script sitting beside them: generate_sample_gppkg.sh writes them
there in buildNative and reads them back in buildGppkg. Anyone who needs
them can run the first step, which is what its README already describes,
so they go.
The rest stay and are recorded instead:
- the sample.gppkg the gppkg behave suite installs, which has to be a
real package because the tests exercise package handling
- src/bin/pgevent/MSG00001.bin, which comes from PostgreSQL, is
referenced by pgmsgevent.rc, and is described by the README next to
it. Cloudberry never builds it: src/bin/Makefile only adds pgevent
to SUBDIRS when PORTNAME is win32
Widening the check matters as much as the deletion. It looked at class,
jar, tar, tgz, zip, exe, dll, so, gz and bz2, so none of the files the
tool found would have tripped it, including the two removed here. It now
covers deb, rpm, gppkg and bin as well, with the two kept files
allowlisted.
README.apache.md gains a section giving the reason for each allowlisted
file, and records the PAX Python API test data
(contrib/pax_storage/src/api/python3/test/test.file*). Those are
PAX-format fixtures read by paxpy_test.py, one per set of column types.
Their names carry no extension the check can match, so documenting them
is all that is possible; they are listed so the set is not invisible.
Checked by running the workflow's allowlist logic over the tree: with deb,
rpm, gppkg and bin added, every matching file is accounted for, and every
allowlist entry still exists.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
cmake/Gpdb.cmake queries pg_config through exec_program(), which is
deprecated. CMake 3.30 added CMP0153 for it, so each of the eleven calls
now prints a developer warning:
CMake Warning (dev) at cmake/Gpdb.cmake:30 (exec_program):
Policy CMP0153 is not set: The exec_program command should not be called.
Run "cmake --help-policy CMP0153" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
Use execute_process() instead.
Call Stack (most recent call first):
CMakeLists.txt:15 (include)
This warning is for project developers. Use -Wno-dev to suppress it.
Route the queries through a pg_config_var() macro over execute_process().
The one behavioural difference that matters is that execute_process()
keeps the trailing newline where exec_program() stripped it, so
OUTPUT_STRIP_TRAILING_WHITESPACE is required; without it every path would
carry a newline into include_directories() and the libpq link line.
Today the warning is only noise, since configure still succeeds. It stops
being noise if the declared minimum ever reaches 3.30, because CMP0153's
NEW behaviour turns exec_program() into a hard error.
Verified on CMake 3.30.5, where the warning appears: all eleven warnings
go away, and the fourteen variables Gpdb.cmake derives, PG_INCLUDE_DIR
through GP_VERSION, come out byte-identical before and after. diskquota
still builds clean on Rocky 8 with CMake 3.26.5.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
The RAT config excused five whole file types from the licence scan, so a clean run said nothing about them. Justin Mclean raised this during the 2.2.0-rc1 IPMC vote. - `**/*.gif` and `**/*.json` match nothing that fails, so they just go. - `**/*.md`, `**/*.sql` and `**/*.out` do hide real gaps: dropping them reports 1088 unapproved files. Almost all are inherited sources that must not be given an ASF header, so they are named instead. For markdown that is README files, orafce's docs, ORCA's design notes and the PR template; for SQL and regression output it is 77 directories plus four files, most of which exist verbatim in PostgreSQL `REL_16_STABLE` or `REL_14_STABLE` or sit in third-party subtrees already declared in LICENSE. - The directory patterns are scoped to .sql and .out rather than written as `<dir>/**`, which would also excuse the C and Perl sources beside them -- 2294 files tree-wide, a wider hole than the globs being replaced. - Project-authored files are unaffected: 36 of 75 .md and 5354 of 6386 `.sql/.out` already carry a header and stay in the scan. A new file outside these paths now has to carry one too. - The gpcloud exclusion gains a comment recording why it stays: those sources came from Greenplum without a header, so an ASF one must not be added. Verified on Rocky 9 against a fresh checkout of main: `mvn clean verify -Drat.consoleOutput=true` gives Unapproved: 0, unknown: 0, generated: 1, approved: 5665 licenses, BUILD SUCCESS. The approved count rises from 5649 because files the globs used to skip are now checked and pass. Assisted-by: Claude Code Backpatch-through: REL_2_STABLE
This script managed a legacy self-hosted S3 RPM repository (sync, GPG-sign RPMs, update/sign repo metadata, upload back to S3). It was carried over as-is from the cloudberry-devops-release repo migration and has had no callers. Removing it as dead code.
…oles pg_file_write(text,text,bool), pg_file_rename(text,text,text), pg_file_unlink(text) and pg_logdir_ls() had proacl = NULL, i.e. PUBLIC EXECUTE. Any role, with no GRANT at all, could create, overwrite, rename or delete files under the data directory and the log directory, and list the log directory. postgresql.auto.conf is writable that way, which turns into code execution as the postgres OS user through shared_preload_libraries or archive_command after a reload or restart. The catalog entries for these functions point at the _v1_1 C symbols, whose bodies deliberately carry no privilege check: they were written for contrib/adminpack, where every CREATE FUNCTION is immediately followed by a REVOKE EXECUTE FROM PUBLIC (adminpack--1.1--2.0.sql). The bodies were brought into core in genfile.c, but neither the REVOKE nor an equivalent in-function check came along. The path confinement in convert_and_check_filename() is a read-side check by its own definition and never covered this. Add requireWriteServerFiles() for the three write-side functions and requireReadServerFiles() for pg_logdir_ls(), mirroring the pg_read_server_files check that convert_and_check_filename() already does; both accept superusers. These take effect as soon as the new binary is in place, which matters because an in-place upgraded cluster keeps proacl = NULL forever. Also add the matching REVOKE and GRANT in system_functions.sql, so a freshly initdb'd cluster is protected at the ACL layer too. gp_toolkit.gp_move_orphaned_files, the only in-tree caller, is run by administrators and is unaffected, and contrib/adminpack keeps its own copies of these C functions. The new genfile_privileges test covers both layers: a plain role is rejected by the ACL, and it is still rejected by genfile.c once it has been granted EXECUTE explicitly, while pg_write_server_files and pg_read_server_files members and superusers are allowed.
s3conf.cpp includes the OpenSSL headers through s3common_headers.h before
it includes c.h, so pg_config.h redefines OPENSSL_API_COMPAT with a
different value than the one OpenSSL already settled on. On Rocky 8
(gcc 8, OpenSSL 1.1.1) every build of the module prints:
In file included from ../../src/include/c.h:56,
from src/s3conf.cpp:16:
../../src/include/pg_config.h:904: warning: "OPENSSL_API_COMPAT" redefined
#define OPENSSL_API_COMPAT 0x10001000L
In file included from /usr/include/openssl/opensslconf.h:42,
from /usr/include/openssl/hmac.h:13,
from include/s3common_headers.h:8,
from include/gpcommon.h:4,
from include/s3conf.h:4,
from src/s3conf.cpp:1:
/usr/include/openssl/opensslconf-x86_64.h:145: note: this is the location of the previous definition
# define OPENSSL_API_COMPAT OPENSSL_MIN_API
Undefine it before including c.h. By then the OpenSSL headers are fully
parsed, so the macro no longer affects anything and behaviour is
unchanged -- only the warning goes away. Reordering the includes so that
c.h comes first would also work, but it would move the extern "C" block
ahead of the C++ standard headers pulled in by s3common_headers.h.
gpcloud.cpp is the only other file in the module that includes a
PostgreSQL header, and it includes postgres.h on its first line, ahead
of any OpenSSL header, so it does not have the problem.
Assisted-by: Claude Code
Backpatch-through: REL_2_STABLE
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
This broad release backport includes unresolved Windows build, CMake compatibility, file-role access, and unit-test issues.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
This PR backports September fixes from main to REL_2_STABLE, covering database behavior, build tooling, and release compliance.
Changes:
- Fix planner, indexing, PAX deletion, and server-file privilege behavior, with regression coverage.
- Update dependencies and build configuration; remove obsolete vendored and Windows installer files.
- Refresh license records, export notice, and source-audit rules.
| File | Description |
|---|---|
src/test/regress/sql/genfile_privileges.sql |
Adds file-function privilege tests. |
src/test/regress/sql/bfv_planner.sql |
Adds outer-join planner regression. |
src/test/regress/parallel_schedule |
Schedules privilege tests. |
src/test/regress/expected/genfile_privileges.out |
Records expected privilege results. |
src/test/regress/expected/bfv_planner.out |
Records planner test output. |
src/test/regress/expected/bfv_planner_optimizer.out |
Records optimizer test output. |
src/test/isolation2/sql/resgroup/resgroup_transaction.sql |
Tests concurrent index creation. |
src/test/isolation2/expected/resgroup/resgroup_transaction.out |
Records index test output. |
src/port/glob.h |
Removes bundled glob header. |
src/port/glob.c |
Removes bundled glob implementation. |
src/include/port/win32_msvc/glob.h |
Removes Windows glob header. |
src/bin/gpfdist/Makefile |
Drops Windows glob object. |
src/bin/gpfdist/CMakeLists.txt |
Drops glob source from target. |
src/backend/utils/adt/genfile.c |
Adds file-function role checks. |
src/backend/optimizer/path/costsize.c |
Clamps join selectivities. |
src/backend/optimizer/path/clausesel.c |
Corrects range-pair null adjustment. |
src/backend/libpq/sha2.h |
Removes stale SHA header. |
src/backend/commands/indexcmds.c |
Clears a catalog snapshot before index phases. |
src/backend/catalog/system_functions.sql |
Sets file-function execution privileges. |
README.md |
Adds crypto export notice. |
README.apache.md |
Documents binary-file exceptions. |
python-dependencies.txt |
Updates Python dependency pins. |
pom.xml |
Narrows license-audit exclusions. |
NOTICE |
Adds Greenplum attribution. |
licenses/LICENSE-pg-hint-plan.txt |
Adds pg_hint_plan license text. |
licenses/LICENSE-pexpect.txt |
Adds pexpect license text. |
licenses/LICENSE-ini.txt |
Corrects ini copyright attribution. |
licenses/LICENSE-http-parser.txt |
Adds HTTP parser license text. |
licenses/LICENSE-glob.txt |
Removes unused glob license text. |
LICENSE |
Updates third-party inventory. |
gpMgmt/test/behave/mgmt_utils/steps/replication_slots_utils.py |
Corrects Python string escapes. |
gpMgmt/test/behave/mgmt_utils/steps/mgmt_utils.py |
Corrects test-string escapes. |
gpMgmt/test/behave/mgmt_utils/steps/gpstate_utils.py |
Corrects a regex escape. |
gpMgmt/test/behave_utils/utils.py |
Corrects a regex escape. |
gpMgmt/sbin/seg_update_pg_hba.py |
Corrects a regex escape. |
gpMgmt/bin/Makefile |
Updates bundled dependency versions. |
gpMgmt/bin/lib/pexpect/pxssh.py |
Corrects prompt-string escapes. |
gpMgmt/bin/gppylib/util/ssh_utils.py |
Corrects a docstring escape. |
gpMgmt/bin/gppylib/test/unit/test_unit_package.py |
Corrects a test regex. |
gpMgmt/bin/gppylib/test/unit/test_unit_gpsegsetuprecovery.py |
Corrects test regexes. |
gpMgmt/bin/gppylib/test/unit/test_unit_gpsegrecovery.py |
Changes recovery-test expectations. |
gpMgmt/bin/gppylib/test/unit/test_unit_gppkg.py |
Corrects a test regex. |
gpMgmt/bin/gppylib/test/unit/test_unit_file_segment_guc.py |
Corrects a test regex. |
gpMgmt/bin/gppylib/test/unit/test_unit_database_segment_guc.py |
Corrects a test regex. |
gpMgmt/bin/gppylib/programs/test/unit/test_cluster_clsrecoversegment_triples.py |
Corrects test regexes. |
gpMgmt/bin/gpload.py |
Corrects regex-string escapes. |
gpMgmt/bin/gpload_test/gpload/TEST.py |
Corrects test-string escapes. |
gpcontrib/gpcloud/src/s3conf.cpp |
Avoids an OpenSSL macro warning. |
gpcontrib/diskquota/CMakeLists.txt |
Lowers declared CMake minimum. |
gpcontrib/diskquota/cmake/Regress.cmake |
Replaces a newer CMake directory variable. |
gpcontrib/diskquota/cmake/Gpdb.cmake |
Modernizes pg_config probing. |
gpAux/Makefile |
Corrects client script name. |
gpAux/client/scripts/cloudberry_clients_path.bat |
Removes obsolete Windows script. |
gpAux/client/install/src/windows/Makefile |
Removes Windows installer build file. |
gpAux/client/install/src/windows/greenplum-clients.wxs |
Removes Windows installer definition. |
gpAux/client/install/src/windows/CreatePackage.bat |
Removes Windows packaging script. |
gpAux/client/install/src/windows/CopyDependencies.bat |
Removes Windows dependency script. |
devops/tools/s3-repo-sync-and-sign.sh |
Removes unused repository script. |
contrib/udp2/ic_common/udp2/ic_udp2_internal.hpp |
Widens history-table purge index. |
contrib/pax_storage/src/cpp/storage/pax.cc |
Projects bloom-filter columns during deletion. |
contrib/pax_storage/src/cpp/cmake/pax.cmake |
Uses fetched yyjson source path. |
contrib/pax_storage/sql/delete_bloom_stats.sql |
Adds PAX deletion regression. |
contrib/pax_storage/pax_schedule |
Schedules PAX regression. |
contrib/pax_storage/expected/delete_bloom_stats.out |
Records PAX regression output. |
contrib/pax_storage/doc/README.md |
Updates yyjson build guidance. |
contrib/pax_storage/CMakeLists.txt |
Fetches yyjson when needed. |
.gitmodules |
Removes yyjson submodule entry. |
.gitignore |
Ignores port locks and thumbnail caches. |
.github/workflows/apache-rat-audit.yml |
Widens binary-file audit. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
leborchuk
marked this pull request as ready for review
September 30, 2026 10:57
Replacing the blanket **/*.md exclude with targeted globs left the docs of the googletest, googlebench and tabulate submodules under contrib/pax_storage uncovered: 36 files that RAT flags on the release tarball, which includes submodules. CI did not notice because the workflow checks out without them. Assisted-by: Claude Code
The targeted .sql/.out excludes that replaced the blanket **/*.sql and **/*.out globs were written against main, which has no yezzey. On this branch that left nine .sql and .out files of the yezzey submodule uncovered, which RAT flags on the release tarball. Exclude them like the other third-party extensions' SQL and expected output. Assisted-by: Claude Code
The release tarball includes submodules, but the RAT workflow audits a checkout without them, so files the tarball ships are never checked. Check out submodules recursively so CI audits the same tree. Backpatch-through: REL_2_STABLE Assisted-by: Claude Code
The README said Cloudberry evolves from "the open-source version of the Pivotal Greenplum Database". That is inaccurate: Pivotal was acquired by VMware in 2019, before Cloudberry was forked in 2022, and "Pivotal Greenplum" named the commercial distribution rather than the open-source project. Refer to it as "the open-source Greenplum® Database" instead, without a company name or version. Also place the registered-trademark sign right after "Greenplum" in both mentions in the introduction, using the plain U+00AE character without the trailing emoji variation selector (U+FE0F). Backpatch-through: REL_2_STABLE Assisted-by: Claude Code
Member
|
@leborchuk I pushed four commits on top of your branch to keep REL_2_STABLE's
With these, RAT on a clean export of this branch with all submodules reports |
Member
|
@leborchuk A few more commits from main that look worth including here. All
|
…ibution gp_create_table_random_default_distribution is a GUC variable and it is declared in guc.h. There is no need to declare it a second time in cdbvars.h.
Suppress false GCC 14 array bounds warnings in protobuf 3.19 headers. Limit the suppression to protobuf includes and restore diagnostics for PAX code. See: apache#1677
…covery The test sets shared_preload_libraries and restarts with "gpstop -raiq". An immediate shutdown skips the shutdown checkpoint, so the control file is left in a state other than DB_SHUTDOWNED and the next startup performs crash recovery: xlogrecovery.c sets InRecovery, xlog.c calls PerformWalRecovery(), which signals PMSIGNAL_RECOVERY_STARTED, and the postmaster moves to PM_RECOVERY. In that state canAcceptConnections() answers CAC_NOTCONSISTENT, reported as "the database system is not accepting connections" with detail "Hot standby mode is disabled". gpstart makes exactly such a connection right after pg_ctl returns, to read the segment configuration, so gpstop -r exits CRITICAL and the restart is reported as failed. The damage does not stop there. psql gives up at the \c that follows, so every statement in the file is skipped and the test fails as a whole; the cleanup at the end of the file never runs; and gpstart never got past starting the coordinator in admin mode, so the cluster is left with no segments up. Suites that run after this one in the same job then lose their Gather Motion nodes and fail as well. Shut down fast instead. A fast shutdown writes the shutdown checkpoint, the control file says DB_SHUTDOWNED, no recovery runs, PM_RECOVERY is never entered, and CAC_NOTCONSISTENT cannot be returned -- the failure becomes unreachable rather than merely less likely. Fast is also what the rest of the tree already uses: gpstop -raf/-arf appear in dozens of places, and this file was the only user of -raiq. Measured on a three-segment demo cluster, dirtying 1.5M coordinator rows before each restart so that recovery is slow enough to lose the race reliably: -raiq failed 2/2 with the message above, -rafq passed 3/3 with all three segments still up afterwards. pg_controldata confirms the mechanism at the other end -- "in production" after an immediate shutdown, "shut down" after a fast one. The test still passes under pg_regress with the change.
…loss advance method Co-authored-by: 王平10304955 <wang.ping20@zte.com.cn>
Contributor
Author
You are right, I missed them since focused mainly on license and bugfixes. But they are worth cherry-picking, it's not a new functionality |
tuhaihe
approved these changes
Sep 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick bugfixes and other useful fixes from main to REL_2_STABLE.
New changes started from the 1st September. List of fixes:
Fix assert failure triggered by "CREATE INDEX CONCURRENTLY" (#1845) 867c6a1
Fix diskquota build on CMake 3.16 15d75c6
Pax: fix delete bloom crash (#1755) 5306ce5
fix(planner): fix range-pair NULL adjustment under outer joins and clamp join selectivities e990399
Fix the clients path script name in gpAux/Makefile 0c49c99
udp2: index the cursor IC history table with uint32 in purge() bee2bbd
Fix missing third-party entries in LICENSE 715df78
Remove BSD-4-Clause glob files inherited from Greenplum 1b6f62a
Remove the Windows client installer and Thumbs.db files b2c78c9
Remove the stale copy of sha2.h under src/backend/libpq 1d53d08
Add the Greenplum attribution to NOTICE 14d3351
Upgrade gpMgmt's bundled Python dependencies c4e0888
Fetch yyjson at build time instead of vendoring it 84a3093
Add export control notice for bundled cryptography (#2004) 2b46a45
Fix invalid escape sequences in gpMgmt Python sources be4f679
Drop the prebuilt gppkg samples and widen the binary check 476527f
Fix CMP0153 warnings from diskquota's pg_config probing 4e678d5
Replace the blanket RAT globs with targeted exclusions caa1710
DevOps: Remove unused s3-repo-sync-and-sign.sh script 8fd8ca6
Restrict pg_file_write/rename/unlink and pg_logdir_ls to privileged roles d01cb3c
Fix OPENSSL_API_COMPAT redefinition warning in gpcloud ef8d794
Fix putIntoUnackQueueRing function logic for Intercontect interfaces:loss advance method 81feec7
interconnect: restart in fast mode so the test does not race crash recovery cc4ff98
Fix PAX protobuf array bounds warnings 238feec
Remove duplicated declaration of gp_create_table_random_default_distribution b3f0cea