Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support aggregate property #693

Merged
merged 17 commits into from
Oct 12, 2019
Merged

support aggregate property #693

merged 17 commits into from
Oct 12, 2019

Conversation

zhoney
Copy link
Contributor

@zhoney zhoney commented Sep 20, 2019

implemented: #691

Change-Id: I340d7be28090718537e1a531147f1e1155a1beb5

@zhoney zhoney force-pushed the aggregate branch 2 times, most recently from d0e84bd to 42f7cd2 Compare September 20, 2019 06:21
@codecov
Copy link

codecov bot commented Sep 20, 2019

Codecov Report

Merging #693 into master will increase coverage by 0.04%.
The diff coverage is 85.1%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #693      +/-   ##
============================================
+ Coverage     74.36%   74.41%   +0.04%     
- Complexity     4097     4143      +46     
============================================
  Files           220      221       +1     
  Lines         17928    18091     +163     
  Branches       2573     2591      +18     
============================================
+ Hits          13332    13462     +130     
- Misses         3237     3253      +16     
- Partials       1359     1376      +17
Impacted Files Coverage Δ Complexity Δ
...baidu/hugegraph/backend/store/BackendFeatures.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../baidu/hugegraph/backend/query/ConditionQuery.java 83.48% <ø> (-0.08%) 94 <0> (-1)
...om/baidu/hugegraph/structure/HugeEdgeProperty.java 58.82% <0%> (ø) 7 <1> (+1) ⬆️
.../baidu/hugegraph/structure/HugeVertexProperty.java 57.89% <0%> (-8.78%) 7 <1> (-1)
...ackend/store/cassandra/CassandraStoreProvider.java 100% <100%> (ø) 6 <1> (ø) ⬇️
...u/hugegraph/backend/store/mysql/MysqlFeatures.java 84.21% <100%> (+0.87%) 16 <1> (+1) ⬆️
...u/hugegraph/backend/serializer/TextSerializer.java 87.37% <100%> (+0.12%) 79 <0> (ø) ⬇️
...u/hugegraph/backend/store/hbase/HbaseFeatures.java 84.21% <100%> (+0.87%) 16 <1> (+1) ⬆️
...du/hugegraph/schema/builder/IndexLabelBuilder.java 87.9% <100%> (+0.09%) 85 <0> (ø) ⬇️
.../hugegraph/backend/serializer/TableSerializer.java 88.28% <100%> (+0.48%) 55 <1> (+1) ⬆️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ef07e0...2e34baf. Read the comment docs.

// Update index of vertex(only include props)
this.indexTx.updateVertexIndex(v, false);
this.indexTx.updateLabelIndex(v, false);
// Update function properties
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function

v.removeProperty(key);
}
}
// Add vertex entry without function properties
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Builder calcSum();

Builder calcOld();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

topN

e.removeProperty(key);
}
}
// Add edge entry of OUT and IN without function properties
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

// Update index of vertex(only include props)
this.indexTx.updateVertexIndex(v, false);
this.indexTx.updateLabelIndex(v, false);
// Update function properties
Map<Id, HugeProperty<?>> props = v.getAggregateProperties();
if (props != null && !props.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't return null

// Update index of edge
this.indexTx.updateEdgeIndex(e, false);
this.indexTx.updateLabelIndex(e, false);
Map<Id, HugeProperty<?>> props = e.getAggregateProperties();
if (props != null && !props.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto


Assert.assertThrows(NotAllowException.class, () -> {
schema.propertyKey("aggregateProperty")
.asUuid().valueSingle().calcMin().create();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhoney zhoney force-pushed the aggregate branch 3 times, most recently from 26cb032 to 166cb20 Compare September 21, 2019 09:53
@@ -319,6 +319,9 @@ private void checkFields(Set<Id> propertyIds) {
"Can't build index on undefined property key " +
"'%s' for '%s': '%s'", field,
this.baseType.readableName(), this.baseValue);
E.checkArgument(pkey.aggregateType().isIndexable(),
"The aggregate type %s in not indexable",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is not indexable

import com.baidu.hugegraph.type.define.Cardinality;
import com.baidu.hugegraph.type.define.DataType;
import com.baidu.hugegraph.util.E;

public class PropertyKeyBuilder implements PropertyKey.Builder {

public static final String TOP_N = "~topN";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KEY_TOP_N

if (this.aggregateType.isTopN()) {
Object n = this.userdata.get(TOP_N);
E.checkArgument(n instanceof Integer && (int) n > 0,
"The top n must > 0, but got: %s", n);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The top n must be positive int

}
if (this.cardinality != Cardinality.SINGLE) {
throw new NotAllowException("Not allowed to set aggregate type " +
"'%s'for property key '%s' with " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a space before "for"

if (this.aggregateType.isNumber() &&
!this.dataType.isNumber() && !this.dataType.isDate()) {
throw new NotAllowException(
"Not allowed to set aggregate type '%s'for " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space

HugeType.PROPERTY : HugeType.AGGR_PROPERTY_E;
}

public Object id() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move HugeEdgeProperty.id() and HugeVertexProperty.id() to HugeProperty

E.checkArgument(n instanceof Integer && (int) n > 0,
"The top n must be positive int, but got: %s", n);
}
if (this.cardinality != Cardinality.SINGLE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cardinality of topN should be list

private void checkAggregateProperty(HugeProperty property) {
E.checkArgument(!property.isAggregateType() ||
this.store().features().supportsAggregateProperty(),
"The store of '%s' not support aggregate " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

private void checkAggregateProperty(HugeElement element) {
E.checkArgument(element.getAggregateProperties().isEmpty() ||
this.store().features().supportsAggregateProperty(),
"The store of '%s' not support aggregate " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The %s store does not support aggregate property

@zhoney zhoney force-pushed the aggregate branch 2 times, most recently from 2f647d2 to 2a91ac5 Compare September 25, 2019 12:24
implemented: #691

Change-Id: I340d7be28090718537e1a531147f1e1155a1beb5
Change-Id: Ib32b4bb297bce5963b9da56e2f26fd9822ff82ec
Change-Id: Id955abfca780792511193e1dd96937399905799e
Change-Id: I46f7af706c5d56e5960c2e667b570ca31d6bff98
Change-Id: Ib7f0bfa7bbdf2b01893c97c2f3e0f35503c7dc93
Change-Id: I427eb98356bffaae7266e604de91baef2d1e7a15
Change-Id: I795ed575eb30dc5cd87c7bc2dff173d7ca268279
Change-Id: Iaa3c88102580c79cbbfc6f7e168b03acf2bc8574
Change-Id: If6aec687456432c40dd44007f2644c7bfdb7bc9d
Change-Id: I4b6faf76f3ebff59cca2a973435d0f055a9bc93a
SUM(3, "sum"),
OLD(4, "old");

private byte code = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final

OLD(4, "old");

private byte code = 0;
private String name = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final

Change-Id: I11ea1726ea434cde0a02526ee36b7e1930398265
Change-Id: I4bff43bc4145db4ba63270431129d8637138b19a
Change-Id: I0151cdbf7378b02641aced520cfa6aced45c4e7a
Change-Id: Ib60a1909f7187a06c8c09d331879ae51813cf438
Change-Id: Icc375f2d28c0c126ac9a935bcfbb4d348c4396e0
Change-Id: I0379329c070ce42f2b505236ea03d87e94990c76
Change-Id: I02a1fea3d2fa3d010a57817e047b3604fcf7f5b2
@javeme javeme merged commit 00faffd into master Oct 12, 2019
@javeme javeme deleted the aggregate branch October 12, 2019 09:32
@gwny gwny mentioned this pull request Nov 20, 2019
@zhoney zhoney mentioned this pull request Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants