Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion methods/kmeans/src/pg_gp/kmeans.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ compute_metric(PGFunction inMetricFn, MemoryContext inMemContext, Datum inVec1,
* in execUtils.c
*/

#if GP_VERSION_NUM >= 70000
#if defined(IS_CLOUDBERRY) || GP_VERSION_NUM >= 70000
if(inMemContext->mem_allocated > 50000)
#else
if(inMemContext->allBytesAlloc - inMemContext->allBytesFreed > 50000)
Expand Down
Empty file added requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this empty file needed?

I noticed this when I ran the Apache Release Audit tool (mvn apache-rat:check).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In apache cloudberry, it's not needed. I will remove it.

Empty file.
3 changes: 3 additions & 0 deletions src/config/Ports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ postgres:

greenplum:
name: Greenplum DB

cloudberry:
name: Cloudberry DB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloudberry DB should be Apache Cloudberry

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

17 changes: 11 additions & 6 deletions src/madpack/madpack.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
for port in ports:
portid_list.append(port)

SUPPORTED_PORTS = ('postgres', 'greenplum')
SUPPORTED_PORTS = ('postgres', 'greenplum', 'cloudberry')

# Global variables
portid = None # Target port ID (eg: pg90, gp40)
Expand Down Expand Up @@ -189,7 +189,8 @@ def _run_m4_and_append(schema, maddir_mod_py, module, sqlfile,
'-I' + maddir_madpack,
sqlfile]
if ( (portid == 'postgres') &
(is_rev_gte(get_rev_num(dbver), get_rev_num('14.0'))) ):
(is_rev_gte(get_rev_num(dbver), get_rev_num('14.0'))) or
(portid == 'cloudberry') ):
m4args = ['m4',
'-P',
'-DMADLIB_SCHEMA=' + schema,
Expand Down Expand Up @@ -370,7 +371,7 @@ def _check_db_port(portid):
if portid == 'postgres':
if row[0]['version'].lower().find('greenplum') < 0:
return True
elif portid == 'greenplum':
elif portid == 'greenplum' or portid == 'cloudberry':
return True
return False
# ------------------------------------------------------------------------------
Expand Down Expand Up @@ -1240,6 +1241,7 @@ def create_install_madlib_sqlfile(args, madpack_cmd):
def get_madlib_function_drop_str(schema):

if ((portid == 'greenplum' and is_rev_gte(get_rev_num(dbver), get_rev_num('7.0'))) or
(portid == 'cloudberry') or
(portid == 'postgres')):
case_str = """
CASE p.prokind
Expand Down Expand Up @@ -1344,14 +1346,14 @@ def set_dynamic_library_path_in_database(dbver_split, madlib_library_path):
(portid == 'postgres' and is_rev_gte(dbver_split, get_rev_num('13.0')))):
libdir = libdir.decode()

libdir = libdir.strip()+'/postgresql'
libdir = str(libdir.strip(), encoding='utf-8')+'/postgresql'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing Note: Encountering TypeError: decoding str is not supported when installing on PostgreSQL 14.19.

Root Cause:
For Postgres 13+ (line 1347), libdir is already decoded to a string via .decode(), but line 1349 attempts to decode it again with
str(libdir.strip(), encoding='utf-8'), which fails because you cannot decode a string that's already been decoded.

Recommended Solution:
Ensure libdir is always decoded to a string before line 1349, then simply strip and append the path:

libdir = subprocess.check_output(['pg_config','--libdir'])
if ((portid == 'greenplum' and is_rev_gte(dbver_split, get_rev_num('7.0'))) or
    (portid == 'postgres' and is_rev_gte(dbver_split, get_rev_num('13.0')))):
    libdir = libdir.decode()
else:
    libdir = libdir.decode('utf-8')

libdir = libdir.strip() + '/postgresql'

This ensures libdir is consistently a string for all code paths (older and newer versions), eliminating the type inconsistency that causes the
error.

Request for Review: Please validate this fix works correctly for both:
- Older versions (Postgres <13, Greenplum <7) where subprocess.check_output() returns bytes
- Newer versions (Postgres 13+, Greenplum 7+) where explicit decoding is needed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool!

paths.append(libdir)

paths.append(madlib_library_path)
dynamic_library_path = ':'.join(paths)

if portid == 'greenplum':
if is_rev_gte(dbver_split, get_rev_num('6.0')):
if portid == 'greenplum' or portid == 'cloudberry':
if is_rev_gte(dbver_split, get_rev_num('6.0')) or portid == 'cloudberry':
ret = os.system('gpconfig -c dynamic_library_path -v \'{0}\''.format(dynamic_library_path))
else:
ret = os.system('gpconfig -c dynamic_library_path -v \'\\{0}\''.format(dynamic_library_path))
Expand Down Expand Up @@ -1482,6 +1484,9 @@ def main(argv):
else:
# only need the first two digits for <= 4.3.4
dbver = '.'.join(map(str, dbver_split[:2]))
elif portid == 'cloudberry':
# Assume Cloudberry will stick to semantic versioning
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume Cloudberry will stick to semantic versioning should be Assume Apache Cloudberry will stick to semantic versioning

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

dbver = str(dbver_split[0])
elif portid == 'postgres':
if is_rev_gte(dbver_split, get_rev_num('10.0')):
# Postgres starting 10.0 uses semantic versioning. Hence,
Expand Down
2 changes: 2 additions & 0 deletions src/madpack/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ def get_dbver(con_args, portid):
# for Greenplum the 3rd digit is necessary to differentiate
# 4.3.5+ from versions < 4.3.5
match = re.search("Greenplum[a-zA-Z\s]*(\d+\.\d+\.\d+)", versionStr)
elif portid == 'cloudberry':
match = re.search("Cloudberry[a-zA-Z\s]*(\d+\.\d+\.\d+)", versionStr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Cloudberry[a-zA-Z\s]*(\d+\.\d+\.\d+)" should be "Apache Cloudberry[a-zA-Z\s]*(\d+\.\d+\.\d+)" ?

I am not entirely sure about this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloudberry is enough to achieve our goal, while Apache Cloudberry is more accurate that maybe is better.

return None if match is None else match.group(1)
except Exception:
error_(this, "Failed reading database version", True)
Expand Down
1 change: 1 addition & 0 deletions src/ports/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
add_subdirectory(postgres)
add_subdirectory(greenplum)
add_subdirectory(cloudberry)
19 changes: 19 additions & 0 deletions src/ports/cloudberry/2/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
add_current_cloudberry_version()
19 changes: 19 additions & 0 deletions src/ports/cloudberry/3/CMakeLists.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still no Cloudberry 3.0 yet. So can remove this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apache MADlib should be able to build against both the REL_2_STABLE and main (3.0.0) branches. I believe it is better to keep support for Cloudberry 3.0. As main (3.0) has not released, maybe support for 3.0 can be labelled as experimental.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with ed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
add_current_cloudberry_version()
Loading