sql-parser icon indicating copy to clipboard operation
sql-parser copied to clipboard

Please include cstdint for gcc13

Open toge opened this issue 1 year ago • 4 comments

There are several compilation errors with gcc 13. You need to include cstdint and add the std:: prefix as described in the error message.

This fix is within the scope of the C++ standard and should not affect other compilers.

c++ -std=c++17 -O3 -c -o src/util/sqlhelper.o src/util/sqlhelper.cpp
In file included from src/sql/CreateStatement.h:4,
                 from src/sql/CreateStatement.cpp:1:
src/sql/ColumnType.h:29:34: error: 'int64_t' has not been declared
   29 |   ColumnType(DataType data_type, int64_t length = 0, int64_t precision = 0, int64_t scale = 0);
      |                                  ^~~~~~~
src/sql/ColumnType.h:29:54: error: 'int64_t' has not been declared
   29 |   ColumnType(DataType data_type, int64_t length = 0, int64_t precision = 0, int64_t scale = 0);
      |                                                      ^~~~~~~
src/sql/ColumnType.h:29:77: error: 'int64_t' has not been declared
   29 |   ColumnType(DataType data_type, int64_t length = 0, int64_t precision = 0, int64_t scale = 0);
      |                                                                             ^~~~~~~
src/sql/ColumnType.h:31:3: error: 'int64_t' does not name a type
   31 |   int64_t length;     // Used for, e.g., VARCHAR(10)
      |   ^~~~~~~
src/sql/ColumnType.h:5:1: note: 'int64_t' is defined in header '<cstdint>'; this is probably fixable by adding '#include <cstdint>'
    4 | #include <ostream>
  +++ |+#include <cstdint>
    5 | 
src/sql/ColumnType.h:32:3: error: 'int64_t' does not name a type
   32 |   int64_t precision;  // Used for, e.g., DECIMAL (6, 4) or TIME (5)
      |   ^~~~~~~
src/sql/ColumnType.h:32:3: note: 'int64_t' is defined in header '<cstdint>'; this is probably fixable by adding '#include <cstdint>'
src/sql/ColumnType.h:33:3: error: 'int64_t' does not name a type
   33 |   int64_t scale;      // Used for DECIMAL (6, 4)
      |   ^~~~~~~
src/sql/ColumnType.h:33:3: note: 'int64_t' is defined in header '<cstdint>'; this is probably fixable by adding '#include <cstdint>'

toge avatar Jun 26 '24 02:06 toge

Thanks for the fix. My only issue with this PR is that I am not a fan of prefixing simple basic types with std::. I would work without the prefix, wouldn't it?

Bouncner avatar Jun 26 '24 09:06 Bouncner

@toge : can you check if changing g++/gcc13 to version 14 in the GH workflow already works? I think Ubuntu 24.04 ships GCC 14 or at least has install options for it.

Bouncner avatar Jun 27 '24 13:06 Bouncner

@Bouncner Sorry for not responding to your reply. I haven't had a lot of time recently, but I will try to respond to the points you pointed out within the next couple of weeks.

toge avatar Jul 07 '24 18:07 toge

@Bouncner Sorry for the delay. I changed this PR to include stdint.h instead of cstdint to avoid compilation errors with gcc13 and later without std:: prefix.

This modification will allow compiling with gcc13 without using std:: and without affecting the previous gcc.

Could you review it again?

Can you check if changing g++/gcc13 to version 14 in the GH workflow already works? I think Ubuntu 24.04 ships GCC 14 or at least has install options for it.

I checked sql-parser works fine on gcc14 without this PR on Fedora Linux 40. This PR seems to be only for gcc 13 now.

toge avatar Jul 21 '24 17:07 toge

Hi @toge, I would go ahead and merge the main branch, but since you are working on a fork, I am not able to.

Are you still interested in getting this PR landed? If you don't have the time, I would take it from here.

Bouncner avatar Mar 18 '25 16:03 Bouncner

@Bouncner Sorry for suspending this PR.

Please go a head. I am happy if this library works on much more environment.

toge avatar Mar 18 '25 19:03 toge

Hi @toge, I would go ahead and merge the main branch, but since you are working on a fork, I am not able to.

Are you still interested in getting this PR landed? If you don't have the time, I would take it from here.

Oh ... did not know I can resolve conflicts on my own here.

Bouncner avatar Mar 18 '25 19:03 Bouncner