You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "deepzliu (via GitHub)" <gi...@apache.org> on 2023/03/14 08:45:54 UTC
[GitHub] [arrow] deepzliu opened a new pull request, #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
deepzliu opened a new pull request, #34555:
URL: https://github.com/apache/arrow/pull/34555
Because compression name is very easy to conflicts when using arrow library in others project.
* Closes: #34556
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1475570642
>
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wgtmac commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1479205589
> How to start a discussion on dev@? never do this before.
@deepzliu You can send your question to dev@arrow.apache.org. Here is the reference: https://lists.apache.org/list.html?dev@arrow.apache.org
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu closed pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu closed pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
URL: https://github.com/apache/arrow/pull/34555
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on a diff in pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on code in PR #34555:
URL: https://github.com/apache/arrow/pull/34555#discussion_r1135433596
##########
cpp/src/arrow/util/type_fwd.h:
##########
@@ -45,17 +45,18 @@ struct Scope;
struct Compression {
/// \brief Compression algorithm
+ // prefix 'ACT'(arrow compression type) to avoid nameing conflicts.
enum type {
Review Comment:
There are similar solution, ref: https://github.com/fmtlib/fmt/commit/8ed264fcd49de5716204e77c6293e1a7aef46906
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on a diff in pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on code in PR #34555:
URL: https://github.com/apache/arrow/pull/34555#discussion_r1135406563
##########
cpp/src/arrow/util/type_fwd.h:
##########
@@ -45,17 +45,18 @@ struct Scope;
struct Compression {
/// \brief Compression algorithm
+ // prefix 'ACT'(arrow compression type) to avoid nameing conflicts.
enum type {
Review Comment:
'enum class type {' has the same error if macro definition before, like:`#define SNAPPY 1`.
The error is 'expected identifier before numeric constant'.
Total sample code:
```
#include <iostream>
#define SNAPPY 1
enum class Compression {
/// \brief Compression algorithm
UNCOMPRESSED,
SNAPPY,
GZIP,
BROTLI,
ZSTD,
LZ4,
LZ4_FRAME,
LZO,
BZ2,
LZ4_HADOOP
};
int main() {
std::cout << "UNCOMPRESSED: " << static_cast<int>(Compression::UNCOMPRESSED) << std::endl;
std::cout << "SNAPPY: " << static_cast<int>(Compression::SNAPPY) << std::endl;
return 0;
}
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mapleFU commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1477885050
https://github.com/apache/arrow/pull/34555#issuecomment-1475533675 @deepzliu Sorry for previously misunderstand this patch. Seems that rocksdb uses code like:
```
#ifdef SNAPPY
#include <snappy.h>
#endif
#ifdef ZLIB
#include <zlib.h>
#endif
```
And it conflict with arrow's definition. @kou What's the style for arrow's enum? Can we just use `kZStd` or use the fixing in this patch? And would it effect the C and other languages api?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mapleFU commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1475553266
How do you use arrow compression? Are you using C++ SDK or C SDK? If you're using C++ sdk, the name and namespace would be like `arrow::util::Compression` and `parquet::Compression`. How did you meet conflict here?
Seems that if you code like:
```
using namespace rocksdb;
using namespace parquet;
using namespace arrow::util;
```
It's likely to conflict. But that's not the reason to change the name for it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wgtmac commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1475577979
I don't know how does your project use these third-party libraries. Generally it is good practice to follow this: https://www.appsloveworld.com/cplus/100/232/conflict-caused-by-define-macro-and-enum-using-same-name
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mapleFU commented on a diff in pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #34555:
URL: https://github.com/apache/arrow/pull/34555#discussion_r1135516266
##########
cpp/src/arrow/util/type_fwd.h:
##########
@@ -45,17 +45,18 @@ struct Scope;
struct Compression {
/// \brief Compression algorithm
+ // prefix 'ACT'(arrow compression type) to avoid nameing conflicts.
enum type {
Review Comment:
Why don't:
```
enum class Compression : int32_t {
/// \brief Compression algorithm
UNCOMPRESSED,
SNAPPY,
GZIP,
BROTLI,
ZSTD,
LZ4,
LZ4_FRAME,
LZO,
BZ2,
LZ4_HADOOP
};
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1467646711
* Closes: #34556
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1478008538
In general, we use the Google C++ Style. It says:
https://google.github.io/styleguide/cppguide.html#Enumerator_Names
> Enumerators (for both scoped and unscoped enums) should be named like [constants](https://google.github.io/styleguide/cppguide.html#Constant_Names), not like [macros](https://google.github.io/styleguide/cppguide.html#Macro_Names). That is, use kEnumName not ENUM_NAME.
But our code doesn't follow the style. (It seems that `clang-format` doesn't support it.)
In general, we should use `kEnumName` style but it breaks many APIs. We should discuss this on `dev@arrow.apache.org`. Could you start a discussion on `dev@`?
Anyway, we can avoid the problem by undefing conflicting macros like the followings:
```c++
#include <rocksdb/...>
#undef SNAPPY
#undef ZLIB
#undef ...
#include <arrow/...>
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1478846712
> In general, we use the Google C++ Style. It says:
>
> https://google.github.io/styleguide/cppguide.html#Enumerator_Names
>
> > Enumerators (for both scoped and unscoped enums) should be named like [constants](https://google.github.io/styleguide/cppguide.html#Constant_Names), not like [macros](https://google.github.io/styleguide/cppguide.html#Macro_Names). That is, use kEnumName not ENUM_NAME.
>
> But our code doesn't follow the style. (It seems that `clang-format` doesn't support it.)
>
> In general, we should use `kEnumName` style but it breaks many APIs. We should discuss this on `dev@arrow.apache.org`. Could you start a discussion on `dev@`?
>
> Anyway, we can avoid the problem by undefing conflicting macros like the followings:
>
> ```c++
> #include <rocksdb/...>
>
> #undef SNAPPY
> #undef ZLIB
> #undef ...
>
> #include <arrow/...>
> ```
This is a good solution, but it should not solve some complex project situation, not specific at the moment.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1467646767
:warning: GitHub issue #34556 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1494974889
OK. I made this pull request draft for now.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu closed pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu closed pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
URL: https://github.com/apache/arrow/pull/34555
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1479155500
> @deepzliu I know that what you mean, however, if enum is changed, every user using arrow will not have backward-capability. So we may Could you start a discussion on dev@?
How to start a discussion on dev@? never do this before.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1475576706
> How do you use arrow compression? Are you using C++ SDK or C SDK? If you're using C++ sdk, the name and namespace would be like `arrow::util::Compression` and `parquet::Compression`. How did you meet conflict here?
>
> Seems that if you code like:
>
> ```
> using namespace rocksdb;
> using namespace parquet;
> using namespace arrow::util;
> ```
>
> It's likely to conflict. But that's not the reason to change the name for it.
namespace is not working for the problem.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu closed pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu closed pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
URL: https://github.com/apache/arrow/pull/34555
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on a diff in pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on code in PR #34555:
URL: https://github.com/apache/arrow/pull/34555#discussion_r1135433596
##########
cpp/src/arrow/util/type_fwd.h:
##########
@@ -45,17 +45,18 @@ struct Scope;
struct Compression {
/// \brief Compression algorithm
+ // prefix 'ACT'(arrow compression type) to avoid nameing conflicts.
enum type {
Review Comment:
Ref: https://github.com/fmtlib/fmt/commit/8ed264fcd49de5716204e77c6293e1a7aef46906
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34555: Add prefix 'ACT_' for 'arrow::Compression::type' to avoid nameing conflicts
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1467588688
<!--
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.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #34555:
URL: https://github.com/apache/arrow/pull/34555#discussion_r1135207312
##########
cpp/src/arrow/util/type_fwd.h:
##########
@@ -45,17 +45,18 @@ struct Scope;
struct Compression {
/// \brief Compression algorithm
+ // prefix 'ACT'(arrow compression type) to avoid nameing conflicts.
enum type {
Review Comment:
Can we use `enum class` instead of adding a prefix?
```suggestion
enum class type {
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on a diff in pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on code in PR #34555:
URL: https://github.com/apache/arrow/pull/34555#discussion_r1135372918
##########
cpp/src/arrow/util/type_fwd.h:
##########
@@ -45,17 +45,18 @@ struct Scope;
struct Compression {
/// \brief Compression algorithm
+ // prefix 'ACT'(arrow compression type) to avoid nameing conflicts.
enum type {
Review Comment:
OK, let me try.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mapleFU commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1478888047
@deepzliu I know that what you mean, however, if enum is changed, every user using arrow will not have backward-capability. So we may Could you start a discussion on dev@?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] amol- commented on pull request #34555: GH-34556: [C++] Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "amol- (via GitHub)" <gi...@apache.org>.
amol- commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1494571502
@kou should we close this one until a decision has been made by the ML? (or move it to a draft?) It seems it has been stuck for a few weeks at this point.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] deepzliu commented on pull request #34555: GH-34556: [C++]Add prefix 'ACT_' for 'arrow::Compression::type' to avoid naming conflicts
Posted by "deepzliu (via GitHub)" <gi...@apache.org>.
deepzliu commented on PR #34555:
URL: https://github.com/apache/arrow/pull/34555#issuecomment-1475533675
(My English is not good, with Chinese at the back.)
I describe the problem I encountered in detail. When I used both Arrow and RocksDB in my project, I encountered definition conflicts such as LZ4 and SNAPPY. In RocksDB, it is a macro definition, and in Arrow, it is an enumeration.
I personally think that in practical applications, this kind of scenario may be more common, it，it's worth to solving this problem.
The solutions are as follows:
1) Use some kind of technology to solve it completely with minimal impact, such as the enum class everyone mentioned. According to the my verification results, cannot solve the problem .
2) Modify the macro definitions of other libraries. However, in my opinion, the high maintenance cost and inconvenient usage are not conducive to promotion and application of Arrow library, especially in my case.
3) Add a prefix or suffix to the Arrow library enumeration, such as my PR, the disadvantages are clearly explained;
4) The user ensures that the Arrow header file is referenced at the front of others in the project. The problem with this method is that, a) the quality of developers is very variable and difficult to guarantee; b) if you use the macro definition in the make parameter, there is no solution;
To sum up, I personally think that solutions 3 may be a method that can fundamentally solve this problem.
---
我详细描述下我遇到的问题。在我的项目中同时用到了Arrow和RocksDB时，遇到LZ4、SNAPPY等定义冲突，在RocksDB是宏定义，在Arrow中是枚举。
个人认为，在实际应用中，这种场景可能会比较常见，所以解决这个问题有一定的价值。
能想到的解决方法有如下：
1）使用某种技术彻底解决，且影响最小，如大家提到的enum class。从验证结果来看，无法解决该问题；
2）改掉其他库的宏定义。但个人认为维护成本高、使用不便捷，不利于Arrow库广泛推广与应用，尤其是我遇到的情况；
3）在Arrow库枚举上加前缀或后缀，如本PR，弊端大家说得很清楚了；
4）使用者保证在项目中Arrow头文件引用在最前面。这个方法的问题在于，a）开发者水平参差不齐，很难保证；b）如果使用make参数中的宏定义，则无解；
综上所述，个人认为方法3可能是能根本解决该问题的方法。
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org