-
Notifications
You must be signed in to change notification settings - Fork 527
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
Conversation
d0e84bd
to
42f7cd2
Compare
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
// Update index of vertex(only include props) | ||
this.indexTx.updateVertexIndex(v, false); | ||
this.indexTx.updateLabelIndex(v, false); | ||
// Update function properties |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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(); | ||
|
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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()) { |
There was a problem hiding this comment.
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()) { |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
26cb032
to
166cb20
Compare
@@ -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", |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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 " + |
There was a problem hiding this comment.
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 " + |
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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 " + |
There was a problem hiding this comment.
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 " + |
There was a problem hiding this comment.
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
2f647d2
to
2a91ac5
Compare
implemented: #691 Change-Id: I340d7be28090718537e1a531147f1e1155a1beb5
Change-Id: Id955abfca780792511193e1dd96937399905799e
Change-Id: Ib7f0bfa7bbdf2b01893c97c2f3e0f35503c7dc93
Change-Id: I427eb98356bffaae7266e604de91baef2d1e7a15
Change-Id: I795ed575eb30dc5cd87c7bc2dff173d7ca268279
Change-Id: Iaa3c88102580c79cbbfc6f7e168b03acf2bc8574
Change-Id: I4b6faf76f3ebff59cca2a973435d0f055a9bc93a
SUM(3, "sum"), | ||
OLD(4, "old"); | ||
|
||
private byte code = 0; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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
implemented: #691
Change-Id: I340d7be28090718537e1a531147f1e1155a1beb5