You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by "deshanxiao (via GitHub)" <gi...@apache.org> on 2023/08/17 11:45:38 UTC
[GitHub] [orc] deshanxiao opened a new pull request, #1588: ORC-1483: [Java] Remove obsolete FileMetadata
deshanxiao opened a new pull request, #1588:
URL: https://github.com/apache/orc/pull/1588
### What changes were proposed in this pull request?
This PR is aimed to remove obsolete FileMetadata.
### Why are the changes needed?
For code optimization
### How was this patch tested?
Exist UT
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1297887834
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
+1 for @mystic-lama points. Acutally this PR targets 2.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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1298946390
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
There are two more comments.
1. +1 for another independent deprecation PR for specific discussion about that.
2. Please note that `Deperecated Code` is not a dead code. If there is any usage in the downstream, we may not be able to delete the deprecated code. For example, we recovered the deleted code back.
ORC-677: Add a deprecated legacy constructor SargApplier back (#557)
ORC-676: Add getRawDataSizeFromColIndices back to ReaderImpl (#555)
ORC-671: Add OrcTail.getStripeStatistics back for backward compatiblility (#549)
We had better be very careful even in 2.0.0. Semantic Versioning means it allows breaking changes, but doesn't mean we need to break absolutely. The fundamental purpose of `Semantic Versioning` is to help users.
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1299513354
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
Thank you @dongjoon-hyun @mystic-lama . I will open an independent PR.
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao closed pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao closed pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
URL: https://github.com/apache/orc/pull/1588
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1298946390
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
There are two more comments.
1. +1 for another independent deprecation PR for specific discussion about that.
2. Please note that `Deperecated Code` is not a dead code. If there is any usage in the downstream, we may not able to delete the deprecated code. For example, we recovered the deleted code back.
ORC-677: Add a deprecated legacy constructor SargApplier back (#557)
ORC-676: Add getRawDataSizeFromColIndices back to ReaderImpl (#555)
ORC-671: Add OrcTail.getStripeStatistics back for backward compatiblility (#549)
We had better be very careful even in 2.0.0. Semantic Versioning means it allows breaking changes, but doesn't mean we need to break absolutely. The fundamental purpose of `Semantic Versioning` is to help users.
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1297874808
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
As @mystic-lama pointed, Apache ORC community cannot drop the public API without a deprecation, @deshanxiao .
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] mystic-lama commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "mystic-lama (via GitHub)" <gi...@apache.org>.
mystic-lama commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1297189413
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
Shall we mark this method as deprecated in early releases and then remove in 2.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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1298946390
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
There are two more comments.
1. +1 for another independent deprecation PR for specific discussion about that.
2. Please note that `Deperecated Code` is not a dead code. If there is any usage in the downstream, we may not be able to delete the deprecated code. For example, we recovered the deleted code back for downstream projects.
ORC-677: Add a deprecated legacy constructor SargApplier back (#557)
ORC-676: Add getRawDataSizeFromColIndices back to ReaderImpl (#555)
ORC-671: Add OrcTail.getStripeStatistics back for backward compatiblility (#549)
We had better be very careful even in 2.0.0. Semantic Versioning means it allows breaking changes, but doesn't mean we need to break absolutely. The fundamental purpose of `Semantic Versioning` is to help users.
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] mystic-lama commented on a diff in pull request #1588: ORC-1483: [Java] Remove obsolete FileMetadata
Posted by "mystic-lama (via GitHub)" <gi...@apache.org>.
mystic-lama commented on code in PR #1588:
URL: https://github.com/apache/orc/pull/1588#discussion_r1298526292
##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -350,18 +346,6 @@ public KeyProvider getKeyProvider() {
return keyProvider;
}
- /**
- * @deprecated Use {@link #orcTail(OrcTail)} instead.
- */
- public ReaderOptions fileMetadata(final FileMetadata metadata) {
- fileMetadata = metadata;
- return this;
- }
-
- public FileMetadata getFileMetadata() {
Review Comment:
@deshanxiao What I meant is we need one more PR with this API marked as deprecated in a version that goes out before 2.0.0, so that people can expect the change. I don't have strong opinion on this, given breaking changes are expected in major release. Please choose what is least disruptive. Once we make a decision on this, I can give my +1
A big Thank You for deleting dead code :bowing_man:
--
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: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org