Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation failures for gpdb7. #612

Open
wants to merge 1 commit into
base: madlib2-master
Choose a base branch
from

Conversation

higuoxing
Copy link

This patch fixes 2 compilation errors:

  1. Incomplete type: forget to include the header file <array>
/home/v/workspace/madlib/src/ports/greenplum/dbconnector/../../postgres/dbconnector/Allocator_impl.hpp:78:50: error: 'std::array<long unsigned int, 1> numElements' has incomplete type
   78 |         std::array<std::size_t, BOOST_PP_INC(n)> numElements = {{ \
      |                                                  ^~~~~~~~~~~
  1. snprintf gets expanded to pg_snprintf in PostgreSQL/Greenplum.
In file included from /home/v/.local/gpdb7/include/postgresql/server/c.h:1330,
                 from /home/v/.local/gpdb7/include/postgresql/server/postgres.h:46,
                 from /home/v/workspace/madlib/src/ports/greenplum/dbconnector/dbconnector.hpp:16:
/usr/include/boost/assert/source_location.hpp: In member function 'std::string boost::source_location::to_string() const':
/usr/include/boost/assert/source_location.hpp:97:9: error: 'pg_snprintf' is not a member of 'std'; did you mean 'vsnprintf'?
   97 |         BOOST_ASSERT_SNPRINTF( buffer, ":%lu", ln );
      |         ^~~~~~~~~~~~~~~~~~~~~
/usr/include/boost/assert/source_location.hpp:104:13: error: 'pg_snprintf' is not a member of 'std'; did you mean 'vsnprintf'?
  104 |             BOOST_ASSERT_SNPRINTF( buffer, ":%lu", co );
      |             ^~~~~~~~~~~~~~~~~~~~~

This patch fixes 2 compilation errors:

1. Incomplete type: forget to include the header file `<array>`

```
/home/v/workspace/madlib/src/ports/greenplum/dbconnector/../../postgres/dbconnector/Allocator_impl.hpp:78:50: error: 'std::array<long unsigned int, 1> numElements' has incomplete type
   78 |         std::array<std::size_t, BOOST_PP_INC(n)> numElements = {{ \
      |                                                  ^~~~~~~~~~~
```

2. `snprintf` gets expanded to `pg_snprintf` in PostgreSQL/Greenplum.

```
In file included from /home/v/.local/gpdb7/include/postgresql/server/c.h:1330,
                 from /home/v/.local/gpdb7/include/postgresql/server/postgres.h:46,
                 from /home/v/workspace/madlib/src/ports/greenplum/dbconnector/dbconnector.hpp:16:
/usr/include/boost/assert/source_location.hpp: In member function 'std::string boost::source_location::to_string() const':
/usr/include/boost/assert/source_location.hpp:97:9: error: 'pg_snprintf' is not a member of 'std'; did you mean 'vsnprintf'?
   97 |         BOOST_ASSERT_SNPRINTF( buffer, ":%lu", ln );
      |         ^~~~~~~~~~~~~~~~~~~~~
/usr/include/boost/assert/source_location.hpp:104:13: error: 'pg_snprintf' is not a member of 'std'; did you mean 'vsnprintf'?
  104 |             BOOST_ASSERT_SNPRINTF( buffer, ":%lu", co );
      |             ^~~~~~~~~~~~~~~~~~~~~
```
@orhankislal
Copy link
Contributor

Could you post the details of the environment you see these errors? I just re-tested on RHEL8, RHEL9 and Ubuntu 20 and it compiles as expected. With your commit I do get a new compilation error.

@higuoxing
Copy link
Author

Could you post the details of the environment you see these errors? I just re-tested on RHEL8, RHEL9 and Ubuntu 20 and it compiles as expected. With your commit I do get a new compilation error.

Ah, I'm using ArchLinux. Could you please paste your compilation error with my patch?

@orhankislal
Copy link
Contributor

Here is the error.

[ 65%] Building CXX object src/ports/greenplum/6/CMakeFiles/madlib_greenplum_6.dir/__/__/__/modules/assoc_rules/assoc_rules.cpp.o
In file included from /usr/include/c++/11/array:35,
                 from /tmp/build/27bbdacd/madlib_src/src/ports/greenplum/dbconnector/../../postgres/dbconnector/dbconnector.hpp:121,
                 from /tmp/build/27bbdacd/madlib_src/src/ports/greenplum/dbconnector/dbconnector.hpp:38,
                 from /tmp/build/27bbdacd/madlib_src/src/modules/assoc_rules/assoc_rules.cpp:11:
/usr/include/c++/11/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
   32 | #error This file requires compiler and library support \
      |  ^~~~~

I think I can work around it by adding -DCXX11=True to the cmake command.

@higuoxing
Copy link
Author

Thanks! I didn't test my patch against gpdb6. I'll try to do it.

@orhankislal
Copy link
Contributor

I was able to get cmake/compile done but it causes a means failure on both rhel8/9 and both gp6/7:

SELECT * FROM kmeanspp('kmeans_2d', 'position', 2, 'mad.squared_dist_norm2', 'mad.avg', 2);
psql:/tmp/madlib.gzfupceg/kmeans/kmeans.ic.sql_in.tmp:55: WARNING:  writer gang of current global transaction is lost
psql:/tmp/madlib.gzfupceg/kmeans/kmeans.ic.sql_in.tmp:55: WARNING:  Any temporary tables for this session have been dropped because the gang was disconnected (session id = 975)
psql:/tmp/madlib.gzfupceg/kmeans/kmeans.ic.sql_in.tmp:55: ERROR:  DTX RollbackAndReleaseCurrentSubTransaction dispatch failed
CONTEXT:  PL/Python function "internal_compute_kmeanspp_seeding"
SQL statement "SELECT (
        SELECT mad.internal_compute_kmeanspp_seeding(
            '_madlib_kmeanspp_args', '_madlib_kmeanspp_state',
            textin(regclassout(class_rel_source)), expr_point)
    )"
PL/pgSQL function kmeanspp_seeding(character varying,character varying,integer,character varying,double precision[],double precision) line 81 at assignment
PL/pgSQL function kmeanspp(character varying,character varying,integer,character varying,character varying,integer,double precision,double precision) line 5 at assignment
PL/pgSQL function kmeanspp(character varying,character varying,integer,character varying,character varying,integer) line 5 at assignment

@beeender
Copy link

Here is the error.

[ 65%] Building CXX object src/ports/greenplum/6/CMakeFiles/madlib_greenplum_6.dir/__/__/__/modules/assoc_rules/assoc_rules.cpp.o
In file included from /usr/include/c++/11/array:35,
                 from /tmp/build/27bbdacd/madlib_src/src/ports/greenplum/dbconnector/../../postgres/dbconnector/dbconnector.hpp:121,
                 from /tmp/build/27bbdacd/madlib_src/src/ports/greenplum/dbconnector/dbconnector.hpp:38,
                 from /tmp/build/27bbdacd/madlib_src/src/modules/assoc_rules/assoc_rules.cpp:11:
/usr/include/c++/11/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
   32 | #error This file requires compiler and library support \
      |  ^~~~~

I think I can work around it by adding -DCXX11=True to the cmake command.

https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html

maybe specify the C++ version by set_property(TARGET tgt PROPERTY CXX_STANDARD 11)
Different OS may have different default values.

@xiongzhenglong
Copy link

Dear Madlib Maintainers,

I am writing to raise an issue I encountered while using Madlib. I attempted to execute the following command:

”madpack install -s madlib -p postgres -c postgres@localhost/madlib_db“

However, I found that this command requires a Python 2.x environment. As Python 2.x is no longer maintained and considering that the rest of the Madlib project is based on Python 3.x, this poses a significant compatibility and security issue.

I understand the complexities involved in migrating code from Python 2.x to Python 3.x, but given the end of life for Python 2, it is crucial for the long-term sustainability of the Madlib project.

Therefore, I kindly request that the team consider updating the madpack install command to be compatible with Python 3.x environments. This change would not only align with the current Python standards but also enhance the usability and security of Madlib.

Thank you for your attention to this matter. I appreciate the hard work that goes into maintaining such a valuable project and look forward to a possible update in this regard.

@edespino
Copy link

edespino commented Dec 11, 2023

Dear Madlib Maintainers,

I am writing to raise an issue I encountered while using Madlib. I attempted to execute the following command:

”madpack install -s madlib -p postgres -c postgres@localhost/madlib_db“

However, I found that this command requires a Python 2.x environment. As Python 2.x is no longer maintained and considering that the rest of the Madlib project is based on Python 3.x, this poses a significant compatibility and security issue.

I understand the complexities involved in migrating code from Python 2.x to Python 3.x, but given the end of life for Python 2, it is crucial for the long-term sustainability of the Madlib project.c

Therefore, I kindly request that the team consider updating the madpack install command to be compatible with Python 3.x environments. This change would not only align with the current Python standards but also enhance the usability and security of Madlib.

Thank you for your attention to this matter. I appreciate the hard work that goes into maintaining such a valuable project and look forward to a possible update in this regard.

@xiongzhenglong

Python 3 support was released with MADlib 2.x. You can find it in the following branch: https://github.com/apache/madlib/tree/madlib2-master.

Is it possible you are using the master branch where Python 2 resides?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants