You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/07/23 17:46:04 UTC
[GitHub] [arrow] bkietz opened a new pull request #7820: ARROW-9068: [C++][Dataset] Simplify partitioning interface
bkietz opened a new pull request #7820:
URL: https://github.com/apache/arrow/pull/7820
Format and Parse now operate on a whole path rather than on individual segments.
This will simplify building WritePlans but I've only made minimal changes there (to keep tests passing); follow up in https://issues.apache.org/jira/browse/ARROW-8382
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs closed pull request #7820: ARROW-9068: [C++][Dataset] Simplify partitioning interface
Posted by GitBox <gi...@apache.org>.
kszucs closed pull request #7820:
URL: https://github.com/apache/arrow/pull/7820
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7820: ARROW-9068: [C++][Dataset] Simplify partitioning interface
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7820:
URL: https://github.com/apache/arrow/pull/7820#issuecomment-663146065
https://issues.apache.org/jira/browse/ARROW-9068
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #7820: ARROW-9068: [C++][Dataset] Simplify partitioning interface
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #7820:
URL: https://github.com/apache/arrow/pull/7820#discussion_r461736303
##########
File path: cpp/src/arrow/dataset/partition.h
##########
@@ -59,21 +59,10 @@ class ARROW_DS_EXPORT Partitioning {
/// \brief The name identifying the kind of partitioning
virtual std::string type_name() const = 0;
- /// \brief Parse a path segment into a partition expression
- ///
- /// \param[in] segment the path segment to parse
- /// \param[in] i the index of segment within a path
- /// \return the parsed expression
- virtual Result<std::shared_ptr<Expression>> Parse(const std::string& segment,
- int i) const = 0;
-
- virtual Result<std::string> Format(const Expression& expr, int i) const {
- // FIXME(bkietz) make this pure virtual
- return Status::NotImplemented("formatting paths from ", type_name(), " Partitioning");
- }
-
/// \brief Parse a path into a partition expression
- Result<std::shared_ptr<Expression>> Parse(const std::string& path) const;
+ virtual Result<std::shared_ptr<Expression>> Parse(const std::string& segment) const = 0;
Review comment:
```suggestion
virtual Result<std::shared_ptr<Expression>> Parse(const std::string& path) const = 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #7820: ARROW-9068: [C++][Dataset] Simplify partitioning interface
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #7820:
URL: https://github.com/apache/arrow/pull/7820#issuecomment-665629390
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] fsaintjacques commented on a change in pull request #7820: ARROW-9068: [C++][Dataset] Simplify partitioning interface
Posted by GitBox <gi...@apache.org>.
fsaintjacques commented on a change in pull request #7820:
URL: https://github.com/apache/arrow/pull/7820#discussion_r461724930
##########
File path: cpp/src/arrow/dataset/partition.h
##########
@@ -59,21 +59,10 @@ class ARROW_DS_EXPORT Partitioning {
/// \brief The name identifying the kind of partitioning
virtual std::string type_name() const = 0;
- /// \brief Parse a path segment into a partition expression
- ///
- /// \param[in] segment the path segment to parse
- /// \param[in] i the index of segment within a path
- /// \return the parsed expression
- virtual Result<std::shared_ptr<Expression>> Parse(const std::string& segment,
- int i) const = 0;
-
- virtual Result<std::string> Format(const Expression& expr, int i) const {
- // FIXME(bkietz) make this pure virtual
- return Status::NotImplemented("formatting paths from ", type_name(), " Partitioning");
- }
-
/// \brief Parse a path into a partition expression
- Result<std::shared_ptr<Expression>> Parse(const std::string& path) const;
+ virtual Result<std::shared_ptr<Expression>> Parse(const std::string& segment) const = 0;
Review comment:
Name the parameter path.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org