You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by dkhwangbo <gi...@git.apache.org> on 2016/01/15 09:03:12 UTC
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
GitHub user dkhwangbo opened a pull request:
https://github.com/apache/tajo/pull/944
TAJO-2058: foreach loop can be collapsed with stream api
I replace many foreach loop with stream api.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dkhwangbo/tajo TAJO-2058
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/944.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #944
----
commit 7d6f109e37039304d16d4ea87c26e7d103a508d4
Author: Dongkyu Hwangbo <hw...@gmail.com>
Date: 2016-01-15T06:04:53Z
initial commit
commit 1385953e6096391fcc07b1c64fbdae5e58e616cb
Author: Dongkyu Hwangbo <hw...@gmail.com>
Date: 2016-01-15T07:52:50Z
rollback to fix error
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by dkhwangbo <gi...@git.apache.org>.
Github user dkhwangbo commented on the pull request:
https://github.com/apache/tajo/pull/944#issuecomment-199620219
@eminency Hi! Thanks for your comment. I apply your comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on the pull request:
https://github.com/apache/tajo/pull/944#issuecomment-197701836
Even though they may not exact, I left some comments.
Please check them out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56449510
--- Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/XMLCatalogSchemaManager.java ---
@@ -675,40 +654,38 @@ protected void validateSQLObject(List<SQLObject> queries, SQLObject testQuery) {
}
protected void mergeExistQueries(List<SQLObject> queries) {
- for (SQLObject query: queries) {
+ queries.forEach(query -> {
validateSQLObject(queries, query);
-
+
targetStore.addExistQuery(query);
- }
+ });
}
protected void mergeDropStatements(List<SQLObject> queries) {
- for (SQLObject query: queries) {
+ queries.forEach(query -> {
validateSQLObject(queries, query);
-
+
targetStore.addDropStatement(query);
- }
+ });
}
--- End diff --
I think these two methods can be removed and merged as lambda itself unless it is called from many places.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r50217032
--- Diff: tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java ---
@@ -419,9 +420,7 @@ public Object clone() throws CloneNotSupportedException {
hash.quantity = quantity;
if (specifiers != null) {
hash.specifiers = new ArrayList<>();
- for (PartitionSpecifier specifier : specifiers) {
- hash.specifiers.add(specifier);
- }
+ hash.specifiers.addAll(specifiers.stream().collect(Collectors.toList()));
--- End diff --
Actually in a case like this, I think it's sufficient and more clear.
```java
hash.specifiers.addAll(specifiers);
```
or more simply,
```java
hash.specifiers = new ArrayList<>(specifiers);
```
Anyway, I think stream doesn't matter.
Because it is simple shallow copy, there are many ways.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56449935
--- Diff: tajo-core-tests/src/test/java/org/apache/tajo/client/TestTajoClient.java ---
@@ -175,10 +176,8 @@ public final void testSessionVariables() throws IOException, TajoException, Inte
String prefixName = "key_";
String prefixValue = "val_";
- List<String> unsetList = new ArrayList<>();
- for(Map.Entry<String, String> entry: client.getAllSessionVariables().entrySet()) {
- unsetList.add(entry.getKey());
- }
+ List<String> unsetList = client.getAllSessionVariables().entrySet().stream()
+ .map(Map.Entry<String, String>::getKey).collect(Collectors.toList());
--- End diff --
I think next one is more readable:
```java
List<String> unsetList = new ArrayList<>(client.getAllSessionVariables().keySet());
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by dkhwangbo <gi...@git.apache.org>.
Github user dkhwangbo commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r50226906
--- Diff: tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java ---
@@ -419,9 +420,7 @@ public Object clone() throws CloneNotSupportedException {
hash.quantity = quantity;
if (specifiers != null) {
hash.specifiers = new ArrayList<>();
- for (PartitionSpecifier specifier : specifiers) {
- hash.specifiers.add(specifier);
- }
+ hash.specifiers.addAll(specifiers.stream().collect(Collectors.toList()));
--- End diff --
@eminency Thanks for your comment. I agree with your opinion.
I'll rewrite it overall correlated with a simple shallow copy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56448401
--- Diff: tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java ---
@@ -471,10 +469,7 @@ public Object clone() throws CloneNotSupportedException {
}
}
if (specifiers != null) {
- listPartition.specifiers = new ArrayList<>();
- for (ListPartitionSpecifier specifier : specifiers) {
- listPartition.specifiers.add(specifier);
- }
+ listPartition.specifiers = specifiers.stream().collect(Collectors.toList());
--- End diff --
Same as above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56448372
--- Diff: tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java ---
@@ -418,10 +419,7 @@ public Object clone() throws CloneNotSupportedException {
}
hash.quantity = quantity;
if (specifiers != null) {
- hash.specifiers = new ArrayList<>();
- for (PartitionSpecifier specifier : specifiers) {
- hash.specifiers.add(specifier);
- }
+ hash.specifiers = specifiers.stream().collect(Collectors.toList());
--- End diff --
It is sufficient with:
```java
hash.specifiers = new ArrayList<>(specifiers);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56449210
--- Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/XMLCatalogSchemaManager.java ---
@@ -644,30 +631,22 @@ protected void validatePatch(List<SchemaPatch> patches, SchemaPatch testPatch) {
protected void mergePatches(List<SchemaPatch> patches) {
final List<DatabaseObject> objects = new ArrayList<>();
--- End diff --
I can't understand why this exists.
Please check if it can be removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by dkhwangbo <gi...@git.apache.org>.
Github user dkhwangbo commented on the pull request:
https://github.com/apache/tajo/pull/944#issuecomment-194617140
@eminency Thanks for your comment. Unfortunately, I face with some unsuspected error while travis test. I'm in investigation about it's cause.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56448985
--- Diff: tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/store/XMLCatalogSchemaManager.java ---
@@ -602,18 +597,10 @@ protected void copySchemaInfo(StoreObject sourceStore) {
unorderedObjects.add(object);
}
}
-
- for (DatabaseObject object: orderedObjects) {
- if (object != null) {
- mergedObjects.add(object);
- }
- }
-
- for (DatabaseObject object: unorderedObjects) {
- if (object != null) {
- mergedObjects.add(object);
- }
- }
+
+ orderedObjects.stream().filter(object -> object != null).forEach(mergedObjects::add);
+
+ unorderedObjects.stream().filter(object -> object != null).forEach(mergedObjects::add);
--- End diff --
You can merge two statements into one like this:
```java
Stream.concat(orderedObjects.stream(), unorderedObjects.stream())
.filter(o -> o != null)
.forEach(mergedObjects::add);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56451584
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/query/TaskRequestImpl.java ---
@@ -216,49 +217,42 @@ public Enforcer getEnforcer() {
return this.fetches;
}
- private void initFetches() {
- if (this.fetches != null) {
+ private void initFetches() {
+ if (this.fetches != null) {
return;
}
TaskRequestProtoOrBuilder p = viaProto ? proto : builder;
- this.fetches = new ArrayList<>();
- for(FetchProto fetch : p.getFetchesList()) {
- fetches.add(fetch);
- }
- }
+ this.fetches = p.getFetchesList().stream().collect(Collectors.toList());
--- End diff --
Same as above early.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on a diff in the pull request:
https://github.com/apache/tajo/pull/944#discussion_r56452167
--- Diff: tajo-core/src/main/java/org/apache/tajo/querymaster/Query.java ---
@@ -303,12 +304,7 @@ public QueryHistory getQueryHistory() {
}
private List<StageHistory> makeStageHistories() {
- List<StageHistory> stageHistories = new ArrayList<>();
- for(Stage eachStage : getStages()) {
- stageHistories.add(eachStage.getStageHistory());
- }
-
- return stageHistories;
+ return getStages().stream().map(Stage::getStageHistory).collect(Collectors.toList());
--- End diff --
It can be put into caller directly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-2058: foreach loop can be collapsed with s...
Posted by eminency <gi...@git.apache.org>.
Github user eminency commented on the pull request:
https://github.com/apache/tajo/pull/944#issuecomment-194070707
Time to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---