You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@madlib.apache.org by "orhankislal (via GitHub)" <gi...@apache.org> on 2023/05/19 13:10:45 UTC
[GitHub] [madlib] orhankislal opened a new pull request, #603: Build: Add gppkg v2 support for GP7
orhankislal opened a new pull request, #603:
URL: https://github.com/apache/madlib/pull/603
<!--
Thanks for sending a pull request! Here are some tips for you:
1. Refer to this link for contribution guidelines https://cwiki.apache.org/confluence/display/MADLIB/Contribution+Guidelines
2. Please Provide the Module Name, a JIRA Number and a short description about your changes.
-->
- [ ] Add the module name, JIRA# to PR/commit and description.
- [ ] Add tests for the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] khannaekta commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "khannaekta (via GitHub)" <gi...@apache.org>.
khannaekta commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1202852304
##########
src/ports/greenplum/cmake/FindGreenplum_6.cmake:
##########
@@ -17,3 +17,64 @@
set(_FIND_PACKAGE_FILE "${CMAKE_CURRENT_LIST_FILE}")
include("${CMAKE_CURRENT_LIST_DIR}/FindGreenplum.cmake")
+
+function(add_gppkg GPDB_VERSION GPDB_VARIANT GPDB_VARIANT_SHORT UPGRADE_SUPPORT)
+ string(TOLOWER ${GPDB_VERSION} GPDB_VERSION_LC)
+ string(REPLACE "." "_" VERSION_ "${GPDB_VERSION}")
+
+ # Get information about the rhel version
+ rh_version(RH_VERSION)
+ string(REGEX MATCH "([0-9])" RH_MAJOR_VERSION "${RH_VERSION}")
+
+ file(WRITE "${CMAKE_BINARY_DIR}/deploy/gppkg/${GPDB_VARIANT}_${VERSION_}_gppkg.cmake" "
+ file(MAKE_DIRECTORY
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/BUILD\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/RPMS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg\"
+ )
+
+ set(GPDB_VERSION \"${GPDB_VERSION}\")
+ set(GPDB_VERSION_LC \"${GPDB_VERSION_LC}\")
+ set(GPDB_VARIANT \"${GPDB_VARIANT}\")
+ set(GPDB_VARIANT_SHORT \"${GPDB_VARIANT_SHORT}\")
+ set(UPGRADE_SUPPORT \"${UPGRADE_SUPPORT}\")
+ set(RH_MAJOR_VERSION \"${RH_MAJOR_VERSION}\")
+ string(TOLOWER \"${GPDB_VARIANT}\" PORT_NAME)
+
+ configure_file(
+ madlib.spec.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS/madlib.spec\"
+ )
+ configure_file(
+ gppkg_spec.yml.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg/gppkg_spec.yml\"
+ )
+
+ if(GPPKG_BINARY AND RPMBUILD_BINARY)
Review Comment:
Just realized, I was looking at the main branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] khannaekta commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "khannaekta (via GitHub)" <gi...@apache.org>.
khannaekta commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1201264754
##########
src/ports/postgres/modules/bayes/test/bayes.sql_in:
##########
@@ -392,5 +392,17 @@ $$ language plpgsql;
SELECT install_test_1();
SELECT install_test_2();
SELECT install_test_3();
-SELECT install_test_4();
-SELECT install_test_5();
+
+-- FIXME: ORCA started to fail with GP7 beta3 while accessing bayes created views in these tests
+m4_include(`SQLCommon.m4')
+m4_changequote(`<!', `!>')
+m4_ifdef(<!__POSTGRESQL__!>, <!!>, <!
+SET optimizer = off;
+!>)
+
+ SELECT install_test_4();
Review Comment:
do these test fail with gppkg v2 installation only or with GP7 beta3 in general ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] orhankislal merged pull request #603: Build: Add gppkg v2 support for GP7
Posted by "orhankislal (via GitHub)" <gi...@apache.org>.
orhankislal merged PR #603:
URL: https://github.com/apache/madlib/pull/603
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] khannaekta commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "khannaekta (via GitHub)" <gi...@apache.org>.
khannaekta commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1202844227
##########
src/ports/greenplum/cmake/FindGreenplum_6.cmake:
##########
@@ -17,3 +17,64 @@
set(_FIND_PACKAGE_FILE "${CMAKE_CURRENT_LIST_FILE}")
include("${CMAKE_CURRENT_LIST_DIR}/FindGreenplum.cmake")
+
+function(add_gppkg GPDB_VERSION GPDB_VARIANT GPDB_VARIANT_SHORT UPGRADE_SUPPORT)
+ string(TOLOWER ${GPDB_VERSION} GPDB_VERSION_LC)
+ string(REPLACE "." "_" VERSION_ "${GPDB_VERSION}")
+
+ # Get information about the rhel version
+ rh_version(RH_VERSION)
+ string(REGEX MATCH "([0-9])" RH_MAJOR_VERSION "${RH_VERSION}")
+
+ file(WRITE "${CMAKE_BINARY_DIR}/deploy/gppkg/${GPDB_VARIANT}_${VERSION_}_gppkg.cmake" "
+ file(MAKE_DIRECTORY
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/BUILD\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/RPMS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg\"
+ )
+
+ set(GPDB_VERSION \"${GPDB_VERSION}\")
+ set(GPDB_VERSION_LC \"${GPDB_VERSION_LC}\")
+ set(GPDB_VARIANT \"${GPDB_VARIANT}\")
+ set(GPDB_VARIANT_SHORT \"${GPDB_VARIANT_SHORT}\")
+ set(UPGRADE_SUPPORT \"${UPGRADE_SUPPORT}\")
+ set(RH_MAJOR_VERSION \"${RH_MAJOR_VERSION}\")
+ string(TOLOWER \"${GPDB_VARIANT}\" PORT_NAME)
+
+ configure_file(
+ madlib.spec.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS/madlib.spec\"
+ )
+ configure_file(
+ gppkg_spec.yml.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg/gppkg_spec.yml\"
+ )
+
+ if(GPPKG_BINARY AND RPMBUILD_BINARY)
Review Comment:
Thanks! Was just suggesting it as we have a separate `.cmake` for each version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] orhankislal commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "orhankislal (via GitHub)" <gi...@apache.org>.
orhankislal commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1202316399
##########
src/ports/postgres/modules/bayes/test/bayes.sql_in:
##########
@@ -392,5 +392,17 @@ $$ language plpgsql;
SELECT install_test_1();
SELECT install_test_2();
SELECT install_test_3();
-SELECT install_test_4();
-SELECT install_test_5();
+
+-- FIXME: ORCA started to fail with GP7 beta3 while accessing bayes created views in these tests
+m4_include(`SQLCommon.m4')
+m4_changequote(`<!', `!>')
+m4_ifdef(<!__POSTGRESQL__!>, <!!>, <!
+SET optimizer = off;
+!>)
+
+ SELECT install_test_4();
Review Comment:
I believe they fail with beta3. Even if we compile from source, we still fail these tests. I don't see any connection between ORCA optimizer failing to create a plan (even `explain` crashes the db) and gppkg.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] orhankislal commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "orhankislal (via GitHub)" <gi...@apache.org>.
orhankislal commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1202850805
##########
src/ports/greenplum/cmake/FindGreenplum_6.cmake:
##########
@@ -17,3 +17,64 @@
set(_FIND_PACKAGE_FILE "${CMAKE_CURRENT_LIST_FILE}")
include("${CMAKE_CURRENT_LIST_DIR}/FindGreenplum.cmake")
+
+function(add_gppkg GPDB_VERSION GPDB_VARIANT GPDB_VARIANT_SHORT UPGRADE_SUPPORT)
+ string(TOLOWER ${GPDB_VERSION} GPDB_VERSION_LC)
+ string(REPLACE "." "_" VERSION_ "${GPDB_VERSION}")
+
+ # Get information about the rhel version
+ rh_version(RH_VERSION)
+ string(REGEX MATCH "([0-9])" RH_MAJOR_VERSION "${RH_VERSION}")
+
+ file(WRITE "${CMAKE_BINARY_DIR}/deploy/gppkg/${GPDB_VARIANT}_${VERSION_}_gppkg.cmake" "
+ file(MAKE_DIRECTORY
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/BUILD\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/RPMS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg\"
+ )
+
+ set(GPDB_VERSION \"${GPDB_VERSION}\")
+ set(GPDB_VERSION_LC \"${GPDB_VERSION_LC}\")
+ set(GPDB_VARIANT \"${GPDB_VARIANT}\")
+ set(GPDB_VARIANT_SHORT \"${GPDB_VARIANT_SHORT}\")
+ set(UPGRADE_SUPPORT \"${UPGRADE_SUPPORT}\")
+ set(RH_MAJOR_VERSION \"${RH_MAJOR_VERSION}\")
+ string(TOLOWER \"${GPDB_VARIANT}\" PORT_NAME)
+
+ configure_file(
+ madlib.spec.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS/madlib.spec\"
+ )
+ configure_file(
+ gppkg_spec.yml.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg/gppkg_spec.yml\"
+ )
+
+ if(GPPKG_BINARY AND RPMBUILD_BINARY)
Review Comment:
We do have `FindGreenplum_7.cmake` in the same folder (in `madlib2-master` branch). It was added in a previous commit that added gp7 support.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] khannaekta commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "khannaekta (via GitHub)" <gi...@apache.org>.
khannaekta commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1201260343
##########
src/ports/greenplum/cmake/FindGreenplum_6.cmake:
##########
@@ -17,3 +17,64 @@
set(_FIND_PACKAGE_FILE "${CMAKE_CURRENT_LIST_FILE}")
include("${CMAKE_CURRENT_LIST_DIR}/FindGreenplum.cmake")
+
+function(add_gppkg GPDB_VERSION GPDB_VARIANT GPDB_VARIANT_SHORT UPGRADE_SUPPORT)
+ string(TOLOWER ${GPDB_VERSION} GPDB_VERSION_LC)
+ string(REPLACE "." "_" VERSION_ "${GPDB_VERSION}")
+
+ # Get information about the rhel version
+ rh_version(RH_VERSION)
+ string(REGEX MATCH "([0-9])" RH_MAJOR_VERSION "${RH_VERSION}")
+
+ file(WRITE "${CMAKE_BINARY_DIR}/deploy/gppkg/${GPDB_VARIANT}_${VERSION_}_gppkg.cmake" "
+ file(MAKE_DIRECTORY
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/BUILD\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/RPMS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg\"
+ )
+
+ set(GPDB_VERSION \"${GPDB_VERSION}\")
+ set(GPDB_VERSION_LC \"${GPDB_VERSION_LC}\")
+ set(GPDB_VARIANT \"${GPDB_VARIANT}\")
+ set(GPDB_VARIANT_SHORT \"${GPDB_VARIANT_SHORT}\")
+ set(UPGRADE_SUPPORT \"${UPGRADE_SUPPORT}\")
+ set(RH_MAJOR_VERSION \"${RH_MAJOR_VERSION}\")
+ string(TOLOWER \"${GPDB_VARIANT}\" PORT_NAME)
+
+ configure_file(
+ madlib.spec.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS/madlib.spec\"
+ )
+ configure_file(
+ gppkg_spec.yml.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg/gppkg_spec.yml\"
+ )
+
+ if(GPPKG_BINARY AND RPMBUILD_BINARY)
Review Comment:
Would it be helpful to create a separate file `FindGreenplum_7.cmake`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [madlib] orhankislal commented on a diff in pull request #603: Build: Add gppkg v2 support for GP7
Posted by "orhankislal (via GitHub)" <gi...@apache.org>.
orhankislal commented on code in PR #603:
URL: https://github.com/apache/madlib/pull/603#discussion_r1202311618
##########
src/ports/greenplum/cmake/FindGreenplum_6.cmake:
##########
@@ -17,3 +17,64 @@
set(_FIND_PACKAGE_FILE "${CMAKE_CURRENT_LIST_FILE}")
include("${CMAKE_CURRENT_LIST_DIR}/FindGreenplum.cmake")
+
+function(add_gppkg GPDB_VERSION GPDB_VARIANT GPDB_VARIANT_SHORT UPGRADE_SUPPORT)
+ string(TOLOWER ${GPDB_VERSION} GPDB_VERSION_LC)
+ string(REPLACE "." "_" VERSION_ "${GPDB_VERSION}")
+
+ # Get information about the rhel version
+ rh_version(RH_VERSION)
+ string(REGEX MATCH "([0-9])" RH_MAJOR_VERSION "${RH_VERSION}")
+
+ file(WRITE "${CMAKE_BINARY_DIR}/deploy/gppkg/${GPDB_VARIANT}_${VERSION_}_gppkg.cmake" "
+ file(MAKE_DIRECTORY
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/BUILD\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/RPMS\"
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg\"
+ )
+
+ set(GPDB_VERSION \"${GPDB_VERSION}\")
+ set(GPDB_VERSION_LC \"${GPDB_VERSION_LC}\")
+ set(GPDB_VARIANT \"${GPDB_VARIANT}\")
+ set(GPDB_VARIANT_SHORT \"${GPDB_VARIANT_SHORT}\")
+ set(UPGRADE_SUPPORT \"${UPGRADE_SUPPORT}\")
+ set(RH_MAJOR_VERSION \"${RH_MAJOR_VERSION}\")
+ string(TOLOWER \"${GPDB_VARIANT}\" PORT_NAME)
+
+ configure_file(
+ madlib.spec.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/SPECS/madlib.spec\"
+ )
+ configure_file(
+ gppkg_spec.yml.in
+ \"\${CMAKE_CURRENT_BINARY_DIR}/${GPDB_VERSION}/gppkg/gppkg_spec.yml\"
+ )
+
+ if(GPPKG_BINARY AND RPMBUILD_BINARY)
Review Comment:
This PR is for the madlib2-master so the focus is on GP7+ & PG13+. GP6 has py3 capabilities via an extension so it is deemphasised.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@madlib.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org