jackylk opened a new pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597 ### Why is this PR needed? After ### What changes were proposed in this PR? ### Does this PR introduce any user interface change? - No - Yes. (please explain the change and update document) ### Is any new testcase added? - No - Yes ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
CarbonDataQA1 commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-579954320 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1799/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580106803 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1800/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580139640 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1801/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
ajantha-bhat commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580153938 @jackylk : how to make sure this PR has removed all the redundant exception throwing ? and make sure new PR don't add any new redundant exception ? @lamber-ken : do we have checkstyle rule for this ? we should enable 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. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580164635 I checked it using IntelliJ's Analyzer ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580209672 @ajantha-bhat I do not think checkstyle can check this, since it is needs to understand the API semantic ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580725350 @jackylk Please describe the PR in details. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373503843 ########## File path: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/ColumnarSplitter.java ########## @@ -36,18 +34,16 @@ * * @param keys * @return - * @throws KeyGenException */ - byte[][] generateAndSplitKey(long[] keys) throws KeyGenException; + byte[][] generateAndSplitKey(long[] keys); Review comment: the caller also should handle the throws exception, like: org.apache.carbondata.core.keygenerator.columnar.impl.MultiDimKeyVarLengthVariableSplitGeneratorUnitTest#testGenerateAndSplitKeyAndGetKeyArrayWithActualLogicWithInt ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373504796 ########## File path: core/src/main/java/org/apache/carbondata/core/scan/executor/util/QueryUtil.java ########## @@ -130,10 +129,9 @@ * @param keyOrdinalList * @param generator * @return - * @throws KeyGenException */ - public static byte[] getMaxKeyBasedOnOrinal(List<Integer> keyOrdinalList, KeyGenerator generator) - throws KeyGenException { + public static byte[] getMaxKeyBasedOnOrinal( Review comment: Please optimize it to getMaxKeyBasedOnOrdinal ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373505204 ########## File path: core/src/main/java/org/apache/carbondata/core/scan/executor/util/QueryUtil.java ########## @@ -130,10 +129,9 @@ * @param keyOrdinalList * @param generator * @return - * @throws KeyGenException */ - public static byte[] getMaxKeyBasedOnOrinal(List<Integer> keyOrdinalList, KeyGenerator generator) - throws KeyGenException { + public static byte[] getMaxKeyBasedOnOrinal( Review comment: Please check Orinal in the whole project. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373506785 ########## File path: core/src/main/java/org/apache/carbondata/core/keygenerator/KeyGenerator.java ########## @@ -31,18 +31,16 @@ * * @param keys * @return byte array - * @throws KeyGenException */ - byte[] generateKey(long[] keys) throws KeyGenException; + byte[] generateKey(long[] keys); Review comment: Please check all caller, like: org.apache.carbondata.core.scan.result.iterator.RawResultIterator#convertRow ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373509677 ########## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/ImplicitIncludeFilterExecutorImpl.java ########## @@ -77,7 +73,7 @@ public BitSet isScanRequired(byte[][] blockMaxValue, byte[][] blockMinValue, } @Override - public void readColumnChunks(RawBlockletColumnChunks rawBlockletColumnChunks) throws IOException { + public void readColumnChunks(RawBlockletColumnChunks rawBlockletColumnChunks) { Review comment: Why didn't remove the exception? like: org.apache.carbondata.core.scan.filter.executer.FilterExecuter#readColumnChunks ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 commented on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580758716 How to handle other redundant exceptions? ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
xubo245 edited a comment on issue #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#issuecomment-580758716 How to handle other redundant exceptions? @jackylk ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373532978 ########## File path: core/src/main/java/org/apache/carbondata/core/scan/executor/util/QueryUtil.java ########## @@ -130,10 +129,9 @@ * @param keyOrdinalList * @param generator * @return - * @throws KeyGenException */ - public static byte[] getMaxKeyBasedOnOrinal(List<Integer> keyOrdinalList, KeyGenerator generator) - throws KeyGenException { + public static byte[] getMaxKeyBasedOnOrinal( Review comment: fixed ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373533053 ########## File path: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/ColumnarSplitter.java ########## @@ -36,18 +34,16 @@ * * @param keys * @return - * @throws KeyGenException */ - byte[][] generateAndSplitKey(long[] keys) throws KeyGenException; + byte[][] generateAndSplitKey(long[] keys); Review comment: This exception is removed ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373533344 ########## File path: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/ColumnarSplitter.java ########## @@ -36,18 +34,16 @@ * * @param keys * @return - * @throws KeyGenException */ - byte[][] generateAndSplitKey(long[] keys) throws KeyGenException; + byte[][] generateAndSplitKey(long[] keys); Review comment: I have not check testcase, I think it is ok ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373534965 ########## File path: core/src/main/java/org/apache/carbondata/core/keygenerator/KeyGenerator.java ########## @@ -31,18 +31,16 @@ * * @param keys * @return byte array - * @throws KeyGenException */ - byte[] generateKey(long[] keys) throws KeyGenException; + byte[] generateKey(long[] keys); Review comment: fixed ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3597: [CARBONDATA-3575] Remove redundant exception throws
URL: https://github.com/apache/carbondata/pull/3597#discussion_r373533344 ########## File path: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/ColumnarSplitter.java ########## @@ -36,18 +34,16 @@ * * @param keys * @return - * @throws KeyGenException */ - byte[][] generateAndSplitKey(long[] keys) throws KeyGenException; + byte[][] generateAndSplitKey(long[] keys); Review comment: I have not check testcase, I think it is ok ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
Free forum by Nabble | Edit this page |