Yingyi Bu has posted comments on this change. Change subject: Cleanup storage exceptions ......................................................................
Patch Set 5: (38 comments) https://asterix-gerrit.ics.uci.edu/#/c/1619/5/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml File asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml: PS5, Line 1646: duplicate Need a better error message. "Index duplicate key" -> "Inserting duplicate keys into a dataset" If that's at the hyracks level, then "Inserting duplicate keys into the primary storage" PS5, Line 1829: duplicate Need a better error message. "Index duplicate key" -> "Inserting duplicate keys into a dataset" If that's at the hyracks level, then "Inserting duplicate keys into the primary storage" PS5, Line 6736: input Need a better error message. "Duplicates in load input" -> "Loading duplicate keys into a dataset" If that's at the hyracks level, then "Loading duplicate keys into the primary storage" https://asterix-gerrit.ics.uci.edu/#/c/1619/5/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml File asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml: PS5, Line 7788: Duplicates Need a better error message. "Duplicates in load input" -> "loading duplicate keys into a dataset" If that's at the hyracks level, then "loading duplicate keys into the primary storage" https://asterix-gerrit.ics.uci.edu/#/c/1619/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/ExternalFilesIndexOperatorDescriptor.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/ExternalFilesIndexOperatorDescriptor.java: PS5, Line 133: throw HyracksDataException.create(e); https://asterix-gerrit.ics.uci.edu/#/c/1619/5/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java: PS5, Line 712: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 772: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( HyracksDataException | IOException e) PS5, Line 1007: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( HyracksDataException | IOException e) PS5, Line 1023: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1042: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( HyracksDataException | IOException e) PS5, Line 1114: MetadataException RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1286: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( HyracksDataException | IOException e) PS5, Line 1347: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1377: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1396: results RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( HyracksDataException | IOException e) PS5, Line 1419: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1454: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1474: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1547: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1565: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1609: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1622: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1641: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(IOException e) PS5, Line 1658: RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1680: catch RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1698: } RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1735: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1750: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1773: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch( IOException e) PS5, Line 1833: Exception RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) PS5, Line 1855: e RemoteException will be wrapped by MetadataException? It's in the method signature. How about: catch(HyracksDataException | IOException e) https://asterix-gerrit.ics.uci.edu/#/c/1619/5/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties File hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties: PS5, Line 60: <= less than PS5, Line 66: Faild Failed https://asterix-gerrit.ics.uci.edu/#/c/1619/5/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexCursor.java: PS5, Line 45: moves Moves https://asterix-gerrit.ics.uci.edu/#/c/1619/5/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/PartitionedOnDiskInvertedIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/PartitionedOnDiskInvertedIndex.java: Line 72: ArrayList<IInvertedListCursor> cursorsOrderedByTokens) throws HyracksDataException { > MAJOR SonarQube violation: Fix this? https://asterix-gerrit.ics.uci.edu/#/c/1619/5/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/OrderedIndexExamplesTest.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/OrderedIndexExamplesTest.java: PS5, Line 738: { can you have a unified if condition using || ? https://asterix-gerrit.ics.uci.edu/#/c/1619/5/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/multithread/LSMBTreeTestWorker.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/multithread/LSMBTreeTestWorker.java: PS5, Line 97: } Do you need an if branch that does nothing? Do you miss an else branch? https://asterix-gerrit.ics.uci.edu/#/c/1619/5/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/perf/LSMTreeRunner.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/perf/LSMTreeRunner.java: PS5, Line 185: printStackTrace Is there a need to print the stack trace here since the exception is propagated? -- To view, visit https://asterix-gerrit.ics.uci.edu/1619 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I020d2b4b1f4ae48fc2df0b720e70a1ce95867d34 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-HasComments: Yes
